[PATCH] D152082: [ValueTracking] getUnderlyingObject() look through phi.

[Joshua Cao via Phabricator via llvm-commits]

caojoshua added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5647
 const Value *llvm::getUnderlyingObject(const Value *V, unsigned MaxLookup) {
+  DenseMap<const Value *, const Value *> Visited;
   std::function<const Value *(const Value *, unsigned)> Visit =
----------------
goldstein.w.n wrote:
> How often is the cache hit?
Not much with default `MaxLookup = 6`.

The reason I add this is because with looking through loop PHI's, we will inifinite loop. I don't actually run into this in the test suite, but if I hardcode MaxLookup to default to 0, we infinite loop in 11 InstCombine tests. 


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5673
+            Visited[V] = V;
+            const Value *NewUnderlying = V;
+            for (const Value *IncomingValue : PHI->incoming_values()) {
----------------
goldstein.w.n wrote:
> Shouldn't V startout as PHI->getIncomingValue(0)?
I changed the logic so that we initially set `NewUnderlying` to `getIncomingValue(0)`. This also requires that we check the phi has >0 arguments, or else some JumpThreading tests fail. Its functionally the same.


================
Comment at: llvm/test/CodeGen/AMDGPU/promote-alloca-to-lds-phi.ll:131
 
-; FIXME: This should be promotable. We need to use
-; getUnderlyingObjects when looking at the icmp user.
----------------
I don't understand AMDGPU code, but I believe we solve the FIXME by promoting the alloca.


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/gather_scatter.ll:859
 ; AVX512-NEXT:    [[MIN_ITERS_CHECK:%.*]] = icmp ult i64 [[TMP3]], 8
-; AVX512-NEXT:    br i1 [[MIN_ITERS_CHECK]], label [[VEC_EPILOG_SCALAR_PH:%.*]], label [[VECTOR_MEMCHECK:%.*]]
-; AVX512:       vector.memcheck:
-; AVX512-NEXT:    [[TMP4:%.*]] = shl nsw i64 [[IDX_EXT]], 2
-; AVX512-NEXT:    [[TMP5:%.*]] = add nsw i64 [[TMP4]], -4
-; AVX512-NEXT:    [[TMP6:%.*]] = lshr i64 [[TMP5]], 2
-; AVX512-NEXT:    [[TMP7:%.*]] = shl i64 [[TMP6]], 6
-; AVX512-NEXT:    [[TMP8:%.*]] = add nuw nsw i64 [[TMP7]], 8
-; AVX512-NEXT:    [[UGLYGEP:%.*]] = getelementptr i8, ptr [[DEST:%.*]], i64 [[TMP8]]
-; AVX512-NEXT:    [[TMP9:%.*]] = shl nuw i64 [[TMP6]], 2
-; AVX512-NEXT:    [[TMP10:%.*]] = add i64 [[TMP9]], 4
-; AVX512-NEXT:    [[UGLYGEP1:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[TMP10]]
-; AVX512-NEXT:    [[TMP11:%.*]] = mul nsw i64 [[IDX_EXT]], -4
-; AVX512-NEXT:    [[UGLYGEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[TMP11]]
-; AVX512-NEXT:    [[TMP12:%.*]] = sub i64 [[TMP10]], [[TMP4]]
-; AVX512-NEXT:    [[UGLYGEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[TMP12]]
-; AVX512-NEXT:    [[BOUND0:%.*]] = icmp ult ptr [[DEST]], [[UGLYGEP1]]
-; AVX512-NEXT:    [[BOUND1:%.*]] = icmp ult ptr [[PTR]], [[UGLYGEP]]
-; AVX512-NEXT:    [[FOUND_CONFLICT:%.*]] = and i1 [[BOUND0]], [[BOUND1]]
-; AVX512-NEXT:    [[BOUND04:%.*]] = icmp ult ptr [[DEST]], [[UGLYGEP3]]
-; AVX512-NEXT:    [[BOUND15:%.*]] = icmp ult ptr [[UGLYGEP2]], [[UGLYGEP]]
-; AVX512-NEXT:    [[FOUND_CONFLICT6:%.*]] = and i1 [[BOUND04]], [[BOUND15]]
-; AVX512-NEXT:    [[CONFLICT_RDX:%.*]] = or i1 [[FOUND_CONFLICT]], [[FOUND_CONFLICT6]]
-; AVX512-NEXT:    br i1 [[CONFLICT_RDX]], label [[VEC_EPILOG_SCALAR_PH]], label [[VECTOR_MAIN_LOOP_ITER_CHECK:%.*]]
+; AVX512-NEXT:    br i1 [[MIN_ITERS_CHECK]], label [[VEC_EPILOG_SCALAR_PH:%.*]], label [[VECTOR_MAIN_LOOP_ITER_CHECK:%.*]]
 ; AVX512:       vector.main.loop.iter.check:
----------------
Similar to ARM/iv_pointer.ll changes. We remove the bounds checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152082