[PATCH] D24729: Don't look through addrspacecast in GetPointerBaseWithConstantOffset

[Artur Pilipenko via llvm-commits]

apilipenko added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:2845
@@ -2845,1 +2844,3 @@
+      // NOTE: Looking through a addrspacecast is not legal here since it would
+      // change address spaces (and thus possibly pointer sizes)
       Ptr = cast<Operator>(Ptr)->getOperand(0);
----------------
reames wrote:
> For consistency, you should also change getUnderlyingObject at the same time.
I don't think that we should. According to the description GetUnderlyingObject "strips off any GEP address adjustments and pointer casts from the specified value, returning the original object being addressed".

GetPointerBaseWithConstantOffset:
"Analyze the specified pointer to see if it can be expressed as a base pointer plus a constant offset. Return the base and offset to the caller."

GetUnderlyingObject is just more powerful at looking through casts, because some casts can hinder a base plus an offset representation. The same thing happens to Value::stripPointerCasts and Value::stripAndAccumulateInBoundsConstantOffsets.

================
Comment at: test/Transforms/GVN/PRE/rle.ll:321
@@ -320,3 +320,1 @@
 
-define i8 @coerce_offset0_addrspacecast(i32 %V, i32* %P) {
-  store i32 %V, i32* %P
----------------
reames wrote:
> Hm, loosing this optimization seems unfortunate.  Any chance we can peak through addrspace casts only when the two pointer sizes are equal?
Because addrspaces don't have clearly defined semantic in langref I think we can do this. Although, in this case I'd also go and change Value::stripAndAccumulateInBoundsConstantOffsets to do the same (currently it doesn't look through addrspace casts at all).  


https://reviews.llvm.org/D24729