Skip to content

Commit

Permalink
[OpenMP][IR] Set correct alignment for internal variables
Browse files Browse the repository at this point in the history
OpenMP runtime functions assume the pointers are aligned to sizeof(pointer),
but it is being aligned incorrectly. Fix with the proper alignment in the IR builder.

Reviewed By: tianshilei1992

Differential Revision: https://reviews.llvm.org/D157040
  • Loading branch information
erjin authored and CongzheUalberta committed Aug 11, 2023
1 parent 51a5707 commit c384e79
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 7 deletions.
6 changes: 1 addition & 5 deletions clang/lib/CodeGen/CGOpenMPRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2161,11 +2161,7 @@ Address CGOpenMPRuntime::emitThreadIDAddress(CodeGenFunction &CGF,
llvm::Value *CGOpenMPRuntime::getCriticalRegionLock(StringRef CriticalName) {
std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
std::string Name = getName({Prefix, "var"});
llvm::GlobalVariable *G = OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
llvm::Align PtrAlign = OMPBuilder.M.getDataLayout().getPointerABIAlignment(G->getAddressSpace());
if (PtrAlign > llvm::Align(G->getAlignment()))
G->setAlignment(PtrAlign);
return G;
return OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
}

namespace {
Expand Down
5 changes: 4 additions & 1 deletion llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4493,7 +4493,10 @@ OpenMPIRBuilder::getOrCreateInternalVariable(Type *Ty, const StringRef &Name,
M, Ty, /*IsConstant=*/false, GlobalValue::CommonLinkage,
Constant::getNullValue(Ty), Elem.first(),
/*InsertBefore=*/nullptr, GlobalValue::NotThreadLocal, AddressSpace);
GV->setAlignment(M.getDataLayout().getABITypeAlign(Ty));
const DataLayout &DL = M.getDataLayout();
const llvm::Align TypeAlign = DL.getABITypeAlign(Ty);
const llvm::Align PtrAlign = DL.getPointerABIAlignment(AddressSpace);
GV->setAlignment(std::max(TypeAlign, PtrAlign));
Elem.second = GV;
}

Expand Down
10 changes: 9 additions & 1 deletion llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2744,7 +2744,15 @@ TEST_F(OpenMPIRBuilderTest, CriticalDirective) {
PointerType *CriticalNamePtrTy =
PointerType::getUnqual(ArrayType::get(Type::getInt32Ty(Ctx), 8));
EXPECT_EQ(CriticalEndCI->getArgOperand(2), CriticalEntryCI->getArgOperand(2));
EXPECT_EQ(CriticalEndCI->getArgOperand(2)->getType(), CriticalNamePtrTy);
GlobalVariable *GV =
dyn_cast<GlobalVariable>(CriticalEndCI->getArgOperand(2));
ASSERT_NE(GV, nullptr);
EXPECT_EQ(GV->getType(), CriticalNamePtrTy);
const DataLayout &DL = M->getDataLayout();
const llvm::Align TypeAlign = DL.getABITypeAlign(CriticalNamePtrTy);
const llvm::Align PtrAlign = DL.getPointerABIAlignment(GV->getAddressSpace());
if (const llvm::MaybeAlign Alignment = GV->getAlign())
EXPECT_EQ(*Alignment, std::max(TypeAlign, PtrAlign));
}

TEST_F(OpenMPIRBuilderTest, OrderedDirectiveDependSource) {
Expand Down

0 comments on commit c384e79

Please sign in to comment.