[PATCH] D144569: [Clang][OpenMP] Fix accessing of aligned arrays in offloaded target regions

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2274
+  if (!IsByRef) {
+    if ((Ctx.getTargetInfo().getTriple().isAMDGCN()) ||
+        (Ctx.getTargetInfo().getTriple().isNVPTX())) {
----------------
doru1004 wrote:
> jhuber6 wrote:
> > doru1004 wrote:
> > > jhuber6 wrote:
> > > > Why does this handling need to be different between CPU and GPU offloading? Strictly speaking, I'm not sure why we need the alignment type here since we'd only get improper alignment on primitive types. So I figured that it should only care about the alignment of the type itself in all cases. Maybe someone can correct me on that.
> > > Are you saying that the previous check was not correct?
> > This is the first I've looked at this code, so I don't know what the intention was. But I would assume it's just making sure that the alignment of the `uintptr_t` is sufficient to contain the by-value copy without causing an addressing error. By that logic I figured it would only care about the alignment of the type, not the declaration itself.
> Assuming that what was there before was correct, then you're saying that the Decl type is always the same as Ty. Is that the case?
I figured that we'd only care about the alignment of the type that's being copied. Because it's not like we can mimic the alignment of the variable on the device. We just need to make sure that its alignment is `<= alignof(uintptr_t)`. Maybe I'm wrong there, someone else could chime in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144569