[RewriteStatepointsForGC] Fix an incorrect assertion

[DaniilSuchkov]

Summary

The assertion verifying that a newly computed value matches what is
already cached used stripPointerCasts() to strip bitcasts, however the
values can be not only pointers, but also vectors of pointers. That is
problematic because stripPointerCasts() doesn't handle vectors of
pointers. This patch introduces an ad-hoc utility function to strip all
bitcasts regardless of the value type.

[skatkov]

The change itself looks good to me.

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

1180

Just wonder, don't you want to eliminate the reason for this assert?:)

[reames]

LGTM as well.

[DaniilSuchkov]

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

1180

I guess we can just wait for opaque pointers, then this problem simply won't exist.

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

if (PHINode *BasePHI = dyn_cast<PHINode>(State.getBaseValue())) {
         Value *InVal = PN->getIncomingValue(i);
         BasicBlock *InBB = PN->getIncomingBlock(i);
         if (!BlockToValue.count(InBB))
           BlockToValue[InBB] = getBaseForInput(InVal, InBB->getTerminator());
         else {
 #ifndef NDEBUG
           Value *OldBase = BlockToValue[InBB];
           Value *Base = getBaseForInput(InVal, nullptr);
+
+          // We can't use `stripPointerCasts` instead of this function because
+          // `stripPointerCasts` doesn't handle vectors of pointers.
+          auto StripBitCasts = [](Value *V) -> Value * {
+            while (auto *BC = dyn_cast<BitCastInst>(V))
+              V = BC->getOperand(0);
+            return V;
+          };
           // In essence this assert states: the only way two values
           // incoming from the same basic block may be different is by
           // being different bitcasts of the same value.  A cleanup
           // that remains TODO is changing findBaseOrBDV to return an
           // llvm::Value of the correct type (and still remain pure).
           // This will remove the need to add bitcasts.
-          assert(Base->stripPointerCasts() == OldBase->stripPointerCasts() &&
+          assert(StripBitCasts(Base) == StripBitCasts(OldBase) &&
                  "findBaseOrBDV should be pure!");
 #endif
         }
         Value *Base = BlockToValue[InBB];
         BasePHI->setIncomingValue(i, Base);
       }
     } else if (SelectInst *BaseSI =
                    dyn_cast<SelectInst>(State.getBaseValue())) {

llvm/test/Transforms/RewriteStatepointsForGC/phi-vector-bitcast.ll

-; XFAIL: *
 ; REQUIRES: asserts
 ; RUN: opt < %s -disable-output -passes=rewrite-statepoints-for-gc
 
 ; We shouldn't crash when we encounter a vector phi with more than one input
 ; from the same predecessor.
 define void @foo(<2 x i8 addrspace(1)*> %arg1, i32 %arg2, i1 %arg3, <2 x i64 addrspace(1)*> %arg4) gc "statepoint-example" personality i32* null {
 bb:
   %tmp = bitcast <2 x i8 addrspace(1)*> %arg1 to <2 x i64 addrspace(1)*>