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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2274
+  if (!IsByRef) {
+    if ((Ctx.getTargetInfo().getTriple().isAMDGCN()) ||
+        (Ctx.getTargetInfo().getTriple().isNVPTX())) {
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144569