[PATCH] D140844: [InstCombine] Canonicalize gep of addrspacecast

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2473
+      return replaceInstUsesWith(GEP, NewAC);
+    }
+  }
----------------
nikic wrote:
> arsenm wrote:
> > arsenm wrote:
> > > We probably shouldn't be reassociating if either address space is non-integral. I think that was one of the fixes that never happened for the existing code
> > But yes,  moving the addrspacecast up makes more sense 
> > We probably shouldn't be reassociating if either address space is non-integral.
> 
> Hm, why? I don't think we really consider GEP to operate on the integer representation?
> 
> > But yes, moving the addrspacecast up makes more sense
> 
> I think in that case I'll probably just ignore this discrepancy for now (and update tests with current opaque pointer behavior) and then return to this once typed pointers are removed and we don't have to worry about opposing transforms.
The non-integral GEP could have some special clamping or wrapping behavior in one address space and not the other 


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

https://reviews.llvm.org/D140844