[PATCH] D134548: [NVPTX] Fix a segfault for bitcasted calls with byval params

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp:1429
       // Check if we have call alignment metadata
       if (getAlign(*CI, Idx, Alignment))
         return Align(Alignment);
----------------
ldrumm wrote:
> tra wrote:
> > Would we ever have an instruction other than `Call` which would also carry explicit alignment metadata? 
> allocas, loads, and stores, though I'm not sure if that's what you're asking. Could you expand a little on what you mean? The code here is only dealing with argument alignment so in isolation the answer is "no", but I have a feeling you might be hinting at something deeper ?
If `CallBase` can be an instruction other than `Call`, will we be able to get argument alignment via `getAlign()` and if so, should we?

Another observation here is that previously digging through bitcasts was done under `if ( dyn_cast<CallInst>(CB))` while now it's done for any `CB` instruction. It looks like it should work (getMaybeBitcastedCallee will bail out on other instructions, if it can't get ), but was that done that intentionally? If looking through bitcasts is applicable to the `Call` instruction only, I'd prefer to keep it under the `if` to make it obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134548