[PATCH] D157040: [OpenMP][IR] Set correct alignment for internal variables

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4496
         /*InsertBefore=*/nullptr, GlobalValue::NotThreadLocal, AddressSpace);
-    GV->setAlignment(M.getDataLayout().getABITypeAlign(Ty));
+    llvm::Align TypeAlign = M.getDataLayout().getABITypeAlign(Ty);
+    llvm::Align PtrAlign =
----------------
You can go for the slightly more readable version below 
```
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));
```


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2753-2754
+      M->getDataLayout().getPointerABIAlignment(GV->getAddressSpace());
+  if (PtrAlign.value() > GV->getAlignment())
+    EXPECT_EQ(GV->getAlignment(), PtrAlign.value());
 }
----------------
`GV->getAlignment()` should disappear. Can you use the following lines instead?
```
if(const llvm::MaybeAlign Alignment = GV->getAlign()) {
  EXPECT_EQ(*Alignment, PtrAlign);
}
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157040/new/

https://reviews.llvm.org/D157040