[PATCH] D111276: [ArgumentPromotion] Support opaque pointers

[Josh Stone via Phabricator via llvm-commits]

cuviper marked 2 inline comments as done.
cuviper added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:729-731
+  // If we're in a recursive function, and the argument is a recursive type
+  // where we do load the same type, we could end up infinitely peeling the
+  // function argument. (This might be too pessimistic for opaque pointers.)
----------------
aeubanks wrote:
> cuviper wrote:
> > aeubanks wrote:
> > > we're slightly powering down this transform under opaque pointers right? since previously we'd treat different pointer types as different and not bail out in the recursive check as often.
> > > it'd be nice to have a test for this change
> > I think it goes both ways -- yes, opaque pointers will look equal and be deemed recursive in cases that weren't before. On the other hand, the former code considered it recursive if the type had a self pointer at all, even if that's not used in the promotion. I'll see if I can craft test examples for both.
> if we're going to support opaque pointers here, we should emulate what would happen with opaque pointers and bail out if we recursively see any pointer type. that way when we do the opaque pointer switch, this pass doesn't suddenly power down. I'd rather power it down beforehand and see if anybody complains
I think this is addressed in the new update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111276