ValueTracking: address review

Author: artagnon

llvm/include/llvm/Analysis/ValueTracking.h

@@ -791,9 +791,9 @@ bool onlyUsedByLifetimeMarkers(const Value *V);
 /// droppable instructions.
 bool onlyUsedByLifetimeMarkersOrDroppableInsts(const Value *V);
 
-/// Return true if the instruction is known to be a vector lane-wise operation
-/// i.e. if it doesn't potentially cross vector lanes.
-bool isLanewiseOperation(const Instruction *I);
+/// Return true if the instruction doesn't potentially cross vector lanes. This
+/// is useful to make GVN-replacements for vector types.
+bool isNotCrossLaneOperation(const Instruction *I);
 
 /// Return true if the instruction does not have any effects besides
 /// calculating the result and does not have undefined behavior.

llvm/lib/Analysis/InstructionSimplify.cpp

@@ -4343,7 +4343,7 @@ static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
   if (isa<PHINode>(I))
     return nullptr;
 
-  if (Op->getType()->isVectorTy() && !isLanewiseOperation(I))
+  if (Op->getType()->isVectorTy() && !isNotCrossLaneOperation(I))
     // For vector types, the simplification must hold per-lane, so forbid
     // potentially cross-lane operations like shufflevector.
     return nullptr;

llvm/lib/Analysis/ValueTracking.cpp

@@ -6947,7 +6947,7 @@ bool llvm::onlyUsedByLifetimeMarkersOrDroppableInsts(const Value *V) {
       V, /* AllowLifetime */ true, /* AllowDroppable */ true);
 }
 
-bool llvm::isLanewiseOperation(const Instruction *I) {
+bool llvm::isNotCrossLaneOperation(const Instruction *I) {
   if (auto *II = dyn_cast<IntrinsicInst>(I)) {
     switch (II->getIntrinsicID()) {
     // TODO: expand this list.
@@ -6967,9 +6967,7 @@ bool llvm::isLanewiseOperation(const Instruction *I) {
       return false;
     }
   }
-  auto *Shuffle = dyn_cast<ShuffleVectorInst>(I);
-  return (!Shuffle || Shuffle->isSelect()) &&
-         !isa<CallBase, BitCastInst, ExtractElementInst>(I);
+  return !isa<CallBase, BitCastInst, ShuffleVectorInst, ExtractElementInst>(I);
 }
 
 bool llvm::isSafeToSpeculativelyExecute(const Instruction *Inst,

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

@@ -3630,7 +3630,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
   //  * The select condition is not a vector, or the intrinsic does not
   //    perform cross-lane operations.
   if (isSafeToSpeculativelyExecuteWithVariableReplaced(&CI) &&
-      isLanewiseOperation(II))
+      isNotCrossLaneOperation(II))
     for (Value *Op : II->args())
       if (auto *Sel = dyn_cast<SelectInst>(Op))
         if (Instruction *R = FoldOpIntoSelect(*II, Sel))