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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to