[PATCH] D127504: [NVPTX] Use the mask() operator to initialize packed structs with pointers

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added a comment.

Hi @tra Artem, do you think the said complicated deeper analysis is profitable?



================
Comment at: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp:1197
           if (aggBuffer.numSymbols) {
-            if (static_cast<const NVPTXTargetMachine &>(TM).is64Bit()) {
+            if (Packed) {
+              if (!STI.hasMaskOperator())
----------------
ikudrin wrote:
> tra wrote:
> > What will happen if we have an aggregate with a mix of packed and unpacked fields and we only want to init with a pointer located in the unpacked part of it? Will something like this trigger the error?
> > ```
> > %ti = type <{ i8, i32 }>
> > %to = type { i8, %ti, i32, void ()* }
> > @n = addrspace(1) global %to {
> >     i8 12,
> >     %ti <{ i8 56, i32 78 }>,
> >     i32 34,
> >     void()* @func
> >     }
> > ```
> > 
> > It's one of the test cases below with another pointer added to non-packed top-level struct.
> > 
> Unfortunately, yes, that would trigger the error. On the other hand, supporting that case would require a deeper analysis of the initialization expression itself, not only the type, because there can be `ptrtoint` operators. I'm not sure if we need that complication, considering that CUDA 11.1 is almost 2 years old already.
> Unfortunately, yes, that would trigger the error. On the other hand, supporting that case would require a deeper analysis of the initialization expression itself, not only the type, because there can be `ptrtoint` operators. I'm not sure if we need that complication, considering that CUDA 11.1 is almost 2 years old already.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127504