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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2473
+      return replaceInstUsesWith(GEP, NewAC);
+    }
+  }
----------------
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.


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

https://reviews.llvm.org/D140844