[PATCH] D29211: InferAddressSpaces: Support memory intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Transforms/Scalar/InferAddressSpaces.cpp:669
+
+      // Intrinsic users may see the same pointer operand in multiple
+      // operands. Skip to the next instruction.
----------------
jlebar wrote:
> It's not just intrinsic users, right?  Regular Instructions could have the same problem?
> 
> How would you feel if we restructured this to be
> 
>   Use &U = *I;
>   ++I;
>   while (I != E && I->getUser() == CurUser) ++I;
> 
>   if (updateSimplePtrUse(U) || updateMemIntrinsicUse(U))
>     continue;
> 
>   ...
> 
> This seems to be a consistent level of abstraction, instead of having one function which does an update and the other which just does a check and leaves the update to the caller.  And it's also obvious how to extend this code to handle other cases -- just add a new `updateFoo` function to the if statement.
My definition of "SimplePtrUse" was that it has a single pointer operand that can be trivially replaced, so this was specifically avoiding that problem and treating multi users as the complex case. My patches for icmp/select have that problem, and they are handled separately from updateSimplePtrUse


https://reviews.llvm.org/D29211