jdoerfert added inline comments.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1411
+ CGM.getLLVMContext(), llvm::Attribute::Alignment,
+ CGM.getContext().getTypeAlignInChars(VarTy).getQuantity()));
----------------
This doesn't work. If the type alignment is > 8 the stack won't fulfill it
unless you modify
```
/// Add worst-case padding so that future allocations are properly aligned.
constexpr const uint32_t Alignment = 8;
```
in `openmp/libomptarget/DeviceRTL/src/State.cpp`.
The fact that the state has a fixed alignment right now makes it impossible to
allocate higher aligned types anyway.
Proposal:
Add an argument to _alloc_shared that is the alignment as computed above,
effecitively making it _alloc_shared_aligned. Modify the stack to actually
align the base pointer rather than extend the allocation based on the alignment
passed in. Then any type alignment can be handled, including user aligned types.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1475
+ CGM.getModule(), OMPRTL___kmpc_free_shared),
+ {AddrSizePair.first, AddrSizePair.second});
}
----------------
Not needed. Will cause a warning, no?
================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:5940
+ } else if (MaybeAlign RetAlign = AI.CB->getRetAlign()) {
+ Alignment = max(Alignment, RetAlign);
}
----------------
This is sensible but needs a test. You can even do it without the else for all
allocations. With the proposed changes above alloc_shared would also fall into
the aligned_alloc case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115888/new/
https://reviews.llvm.org/D115888
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
