[llvm] 35dfa78 - [OpenMP][IRBuilder] Handle floats for atomic update and fix AllocaIP for update/capture
Shraiysh Vaishay via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 9 23:46:20 PST 2022
Author: Shraiysh Vaishay
Date: 2022-02-10T13:16:10+05:30
New Revision: 35dfa78ff8d44733b1c805309f0bbd4a8c960897
URL: https://github.com/llvm/llvm-project/commit/35dfa78ff8d44733b1c805309f0bbd4a8c960897
DIFF: https://github.com/llvm/llvm-project/commit/35dfa78ff8d44733b1c805309f0bbd4a8c960897.diff
LOG: [OpenMP][IRBuilder] Handle floats for atomic update and fix AllocaIP for update/capture
This patch fixes `createAtomicUpdate` for lowering with float types.
Test added for the same.
This patch also changes the alloca argument for createAtomicUpdate and
createAtomicCapture from `Instruction*` to `InsertPointTy`. This is in
line with the other functions of the OpenMPIRBuilder class which take
AllocaIP as an `InsertPointTy`.
Reviewed By: Meinersbur
Differential Revision: https://reviews.llvm.org/D118227
Added:
Modified:
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 18a88ecf1388c..7643da21be601 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1214,7 +1214,8 @@ class OpenMPIRBuilder {
/// For complex Operations: X = UpdateOp(X) => CmpExch X, old_X, UpdateOp(X)
/// Only Scalar data types.
///
- /// \param AllocIP Instruction to create AllocaInst before.
+ /// \param AllocaIP The insertion point to be used for alloca
+ /// instructions.
/// \param X The target atomic pointer to be updated
/// \param XElemTy The element type of the atomic pointer.
/// \param Expr The value to update X with.
@@ -1234,7 +1235,7 @@ class OpenMPIRBuilder {
/// \returns A pair of the old value of X before the update, and the value
/// used for the update.
std::pair<Value *, Value *>
- emitAtomicUpdate(Instruction *AllocIP, Value *X, Type *XElemTy, Value *Expr,
+ emitAtomicUpdate(InsertPointTy AllocaIP, Value *X, Type *XElemTy, Value *Expr,
AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
AtomicUpdateCallbackTy &UpdateOp, bool VolatileX,
bool IsXBinopExpr);
@@ -1286,7 +1287,7 @@ class OpenMPIRBuilder {
/// Only Scalar data types.
///
/// \param Loc The insert and source location description.
- /// \param AllocIP Instruction to create AllocaInst before.
+ /// \param AllocaIP The insertion point to be used for alloca instructions.
/// \param X The target atomic pointer to be updated
/// \param Expr The value to update X with.
/// \param AO Atomic ordering of the generated atomic instructions.
@@ -1302,7 +1303,7 @@ class OpenMPIRBuilder {
///
/// \return Insertion point after generated atomic update IR.
InsertPointTy createAtomicUpdate(const LocationDescription &Loc,
- Instruction *AllocIP, AtomicOpValue &X,
+ InsertPointTy AllocaIP, AtomicOpValue &X,
Value *Expr, AtomicOrdering AO,
AtomicRMWInst::BinOp RMWOp,
AtomicUpdateCallbackTy &UpdateOp,
@@ -1317,7 +1318,7 @@ class OpenMPIRBuilder {
/// X = UpdateOp(X); V = X,
///
/// \param Loc The insert and source location description.
- /// \param AllocIP Instruction to create AllocaInst before.
+ /// \param AllocaIP The insertion point to be used for alloca instructions.
/// \param X The target atomic pointer to be updated
/// \param V Memory address where to store captured value
/// \param Expr The value to update X with.
@@ -1338,7 +1339,7 @@ class OpenMPIRBuilder {
///
/// \return Insertion point after generated atomic capture IR.
InsertPointTy
- createAtomicCapture(const LocationDescription &Loc, Instruction *AllocIP,
+ createAtomicCapture(const LocationDescription &Loc, InsertPointTy AllocaIP,
AtomicOpValue &X, AtomicOpValue &V, Value *Expr,
AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
AtomicUpdateCallbackTy &UpdateOp, bool UpdateExpr,
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 476686d65f9b4..293f956b6bce4 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3291,9 +3291,10 @@ OpenMPIRBuilder::createAtomicWrite(const LocationDescription &Loc,
}
OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createAtomicUpdate(
- const LocationDescription &Loc, Instruction *AllocIP, AtomicOpValue &X,
+ const LocationDescription &Loc, InsertPointTy AllocaIP, AtomicOpValue &X,
Value *Expr, AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
AtomicUpdateCallbackTy &UpdateOp, bool IsXBinopExpr) {
+ assert(!isConflictIP(Loc.IP, AllocaIP) && "IPs must not be ambiguous");
if (!updateToLocation(Loc))
return Loc.IP;
@@ -3310,7 +3311,7 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createAtomicUpdate(
"OpenMP atomic does not support LT or GT operations");
});
- emitAtomicUpdate(AllocIP, X.Var, X.ElemTy, Expr, AO, RMWOp, UpdateOp,
+ emitAtomicUpdate(AllocaIP, X.Var, X.ElemTy, Expr, AO, RMWOp, UpdateOp,
X.IsVolatile, IsXBinopExpr);
checkAndEmitFlushAfterAtomic(Loc, AO, AtomicKind::Update);
return Builder.saveIP();
@@ -3345,13 +3346,13 @@ Value *OpenMPIRBuilder::emitRMWOpAsInstruction(Value *Src1, Value *Src2,
}
std::pair<Value *, Value *> OpenMPIRBuilder::emitAtomicUpdate(
- Instruction *AllocIP, Value *X, Type *XElemTy, Value *Expr,
+ InsertPointTy AllocaIP, Value *X, Type *XElemTy, Value *Expr,
AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
AtomicUpdateCallbackTy &UpdateOp, bool VolatileX, bool IsXBinopExpr) {
- bool DoCmpExch =
- ((RMWOp == AtomicRMWInst::BAD_BINOP) || (RMWOp == AtomicRMWInst::FAdd)) ||
- (RMWOp == AtomicRMWInst::FSub) ||
- (RMWOp == AtomicRMWInst::Sub && !IsXBinopExpr);
+ bool DoCmpExch = (RMWOp == AtomicRMWInst::BAD_BINOP) ||
+ (RMWOp == AtomicRMWInst::FAdd) ||
+ (RMWOp == AtomicRMWInst::FSub) ||
+ (RMWOp == AtomicRMWInst::Sub && !IsXBinopExpr) || !XElemTy;
std::pair<Value *, Value *> Res;
if (XElemTy->isIntegerTy() && !DoCmpExch) {
@@ -3381,12 +3382,12 @@ std::pair<Value *, Value *> OpenMPIRBuilder::emitAtomicUpdate(
BasicBlock *ContBB = CurBB->splitBasicBlock(CurBB->getTerminator(),
X->getName() + ".atomic.cont");
ContBB->getTerminator()->eraseFromParent();
+ Builder.restoreIP(AllocaIP);
+ AllocaInst *NewAtomicAddr = Builder.CreateAlloca(XElemTy);
+ NewAtomicAddr->setName(X->getName() + "x.new.val");
Builder.SetInsertPoint(ContBB);
llvm::PHINode *PHI = Builder.CreatePHI(OldVal->getType(), 2);
PHI->addIncoming(OldVal, CurBB);
- AllocaInst *NewAtomicAddr = Builder.CreateAlloca(XElemTy);
- NewAtomicAddr->setName(X->getName() + "x.new.val");
- NewAtomicAddr->moveBefore(AllocIP);
IntegerType *NewAtomicCastTy =
IntegerType::get(M.getContext(), XElemTy->getScalarSizeInBits());
bool IsIntTy = XElemTy->isIntegerTy();
@@ -3408,7 +3409,7 @@ std::pair<Value *, Value *> OpenMPIRBuilder::emitAtomicUpdate(
Value *Upd = UpdateOp(OldExprVal, Builder);
Builder.CreateStore(Upd, NewAtomicAddr);
- LoadInst *DesiredVal = Builder.CreateLoad(XElemTy, NewAtomicIntAddr);
+ LoadInst *DesiredVal = Builder.CreateLoad(IntCastTy, NewAtomicIntAddr);
Value *XAddr =
(IsIntTy)
? X
@@ -3416,7 +3417,7 @@ std::pair<Value *, Value *> OpenMPIRBuilder::emitAtomicUpdate(
AtomicOrdering Failure =
llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(AO);
AtomicCmpXchgInst *Result = Builder.CreateAtomicCmpXchg(
- XAddr, OldExprVal, DesiredVal, llvm::MaybeAlign(), AO, Failure);
+ XAddr, PHI, DesiredVal, llvm::MaybeAlign(), AO, Failure);
Result->setVolatile(VolatileX);
Value *PreviousVal = Builder.CreateExtractValue(Result, /*Idxs=*/0);
Value *SuccessFailureVal = Builder.CreateExtractValue(Result, /*Idxs=*/1);
@@ -3440,7 +3441,7 @@ std::pair<Value *, Value *> OpenMPIRBuilder::emitAtomicUpdate(
}
OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createAtomicCapture(
- const LocationDescription &Loc, Instruction *AllocIP, AtomicOpValue &X,
+ const LocationDescription &Loc, InsertPointTy AllocaIP, AtomicOpValue &X,
AtomicOpValue &V, Value *Expr, AtomicOrdering AO,
AtomicRMWInst::BinOp RMWOp, AtomicUpdateCallbackTy &UpdateOp,
bool UpdateExpr, bool IsPostfixUpdate, bool IsXBinopExpr) {
@@ -3463,7 +3464,7 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createAtomicCapture(
// 'x' is simply atomically rewritten with 'expr'.
AtomicRMWInst::BinOp AtomicOp = (UpdateExpr ? RMWOp : AtomicRMWInst::Xchg);
std::pair<Value *, Value *> Result =
- emitAtomicUpdate(AllocIP, X.Var, X.ElemTy, Expr, AO, AtomicOp, UpdateOp,
+ emitAtomicUpdate(AllocaIP, X.Var, X.ElemTy, Expr, AO, AtomicOp, UpdateOp,
X.IsVolatile, IsXBinopExpr);
Value *CapturedVal = (IsPostfixUpdate ? Result.first : Result.second);
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index f5fd7dea5ccdd..48f720bd2d727 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -2936,7 +2936,8 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicUpdate) {
bool IsXLHSInRHSPart = false;
BasicBlock *EntryBB = BB;
- Instruction *AllocIP = EntryBB->getFirstNonPHI();
+ OpenMPIRBuilder::InsertPointTy AllocaIP(EntryBB,
+ EntryBB->getFirstInsertionPt());
Value *Sub = nullptr;
auto UpdateOp = [&](Value *Atomic, IRBuilder<> &IRB) {
@@ -2944,7 +2945,7 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicUpdate) {
return Sub;
};
Builder.restoreIP(OMPBuilder.createAtomicUpdate(
- Builder, AllocIP, X, Expr, AO, RMWOp, UpdateOp, IsXLHSInRHSPart));
+ Builder, AllocaIP, X, Expr, AO, RMWOp, UpdateOp, IsXLHSInRHSPart));
BasicBlock *ContBB = EntryBB->getSingleSuccessor();
BranchInst *ContTI = dyn_cast<BranchInst>(ContBB->getTerminator());
EXPECT_NE(ContTI, nullptr);
@@ -2982,6 +2983,78 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicUpdate) {
EXPECT_FALSE(verifyModule(*M, &errs()));
}
+TEST_F(OpenMPIRBuilderTest, OMPAtomicUpdateFloat) {
+ OpenMPIRBuilder OMPBuilder(*M);
+ OMPBuilder.initialize();
+ F->setName("func");
+ IRBuilder<> Builder(BB);
+
+ OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+ Type *FloatTy = Type::getFloatTy(M->getContext());
+ AllocaInst *XVal = Builder.CreateAlloca(FloatTy);
+ XVal->setName("AtomicVar");
+ Builder.CreateStore(ConstantFP::get(Type::getFloatTy(Ctx), 0.0), XVal);
+ OpenMPIRBuilder::AtomicOpValue X = {XVal, FloatTy, false, false};
+ AtomicOrdering AO = AtomicOrdering::Monotonic;
+ Constant *ConstVal = ConstantFP::get(Type::getFloatTy(Ctx), 1.0);
+ Value *Expr = nullptr;
+ AtomicRMWInst::BinOp RMWOp = AtomicRMWInst::FSub;
+ bool IsXLHSInRHSPart = false;
+
+ BasicBlock *EntryBB = BB;
+ OpenMPIRBuilder::InsertPointTy AllocaIP(EntryBB,
+ EntryBB->getFirstInsertionPt());
+ Value *Sub = nullptr;
+
+ auto UpdateOp = [&](Value *Atomic, IRBuilder<> &IRB) {
+ Sub = IRB.CreateFSub(ConstVal, Atomic);
+ return Sub;
+ };
+ Builder.restoreIP(OMPBuilder.createAtomicUpdate(
+ Builder, AllocaIP, X, Expr, AO, RMWOp, UpdateOp, IsXLHSInRHSPart));
+ BasicBlock *ContBB = EntryBB->getSingleSuccessor();
+ BranchInst *ContTI = dyn_cast<BranchInst>(ContBB->getTerminator());
+ EXPECT_NE(ContTI, nullptr);
+ BasicBlock *EndBB = ContTI->getSuccessor(0);
+ EXPECT_TRUE(ContTI->isConditional());
+ EXPECT_EQ(ContTI->getSuccessor(1), ContBB);
+ EXPECT_NE(EndBB, nullptr);
+
+ PHINode *Phi = dyn_cast<PHINode>(&ContBB->front());
+ EXPECT_NE(Phi, nullptr);
+ EXPECT_EQ(Phi->getNumIncomingValues(), 2U);
+ EXPECT_EQ(Phi->getIncomingBlock(0), EntryBB);
+ EXPECT_EQ(Phi->getIncomingBlock(1), ContBB);
+
+ EXPECT_EQ(Sub->getNumUses(), 1U);
+ StoreInst *St = dyn_cast<StoreInst>(Sub->user_back());
+ AllocaInst *UpdateTemp = dyn_cast<AllocaInst>(St->getPointerOperand());
+
+ ExtractValueInst *ExVI1 =
+ dyn_cast<ExtractValueInst>(Phi->getIncomingValueForBlock(ContBB));
+ EXPECT_NE(ExVI1, nullptr);
+ AtomicCmpXchgInst *CmpExchg =
+ dyn_cast<AtomicCmpXchgInst>(ExVI1->getAggregateOperand());
+ EXPECT_NE(CmpExchg, nullptr);
+ BitCastInst *BitCastNew =
+ dyn_cast<BitCastInst>(CmpExchg->getPointerOperand());
+ EXPECT_NE(BitCastNew, nullptr);
+ EXPECT_EQ(BitCastNew->getOperand(0), XVal);
+ EXPECT_EQ(CmpExchg->getCompareOperand(), Phi);
+ EXPECT_EQ(CmpExchg->getSuccessOrdering(), AtomicOrdering::Monotonic);
+
+ LoadInst *Ld = dyn_cast<LoadInst>(CmpExchg->getNewValOperand());
+ EXPECT_NE(Ld, nullptr);
+ BitCastInst *BitCastOld = dyn_cast<BitCastInst>(Ld->getPointerOperand());
+ EXPECT_NE(BitCastOld, nullptr);
+ EXPECT_EQ(UpdateTemp, BitCastOld->getOperand(0));
+
+ Builder.CreateRetVoid();
+ OMPBuilder.finalize();
+ EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
TEST_F(OpenMPIRBuilderTest, OMPAtomicCapture) {
OpenMPIRBuilder OMPBuilder(*M);
OMPBuilder.initialize();
@@ -3009,13 +3082,14 @@ TEST_F(OpenMPIRBuilderTest, OMPAtomicCapture) {
bool UpdateExpr = true;
BasicBlock *EntryBB = BB;
- Instruction *AllocIP = EntryBB->getFirstNonPHI();
+ OpenMPIRBuilder::InsertPointTy AllocaIP(EntryBB,
+ EntryBB->getFirstInsertionPt());
// integer update - not used
auto UpdateOp = [&](Value *Atomic, IRBuilder<> &IRB) { return nullptr; };
Builder.restoreIP(OMPBuilder.createAtomicCapture(
- Builder, AllocIP, X, V, Expr, AO, RMWOp, UpdateOp, UpdateExpr,
+ Builder, AllocaIP, X, V, Expr, AO, RMWOp, UpdateOp, UpdateExpr,
IsPostfixUpdate, IsXLHSInRHSPart));
EXPECT_EQ(EntryBB->getParent()->size(), 1U);
AtomicRMWInst *ARWM = dyn_cast<AtomicRMWInst>(Init->getNextNode());
More information about the llvm-commits
mailing list