[PATCH] D104663: [OpaquePtr] Remove checking pointee type for byval/preallocated type

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

I agree with @dblaikie that it'd be nice to get a clear answer for the behaviour (and documenting it in LangRef). Also, I wonder about ignoring the instruction attribute when the function declaration exists... seems like that is a step toward removing the instruction attribute entirely (maybe that's the right thing to do (not sure it is), but if so then it should be a high-level bit, not a side effect).

> Don't worry about preallocated since it's still not really in use, and
> is much more involved in setting up, making the chance of a mismatch
> very small.

Not sure I agree with the logic here, although I could be misunderstanding it. If this is a supported IR feature, seems like knowingly inserting a bug wouldn't be great... is that what's happening? (Maybe adding the context @dblaikie mentioned would clarify for me...)



================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1731
   Type *getParamByValType(unsigned ArgNo) const {
     Type *Ty = Attrs.getParamByValType(ArgNo);
+    if (const Function *F = getCalledFunction())
----------------
Is it intentional that `Ty` is not used at all by the early return? If so, it'd be more clear to move this to `getParamByValType` below the `if` statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104663