[PATCH] D53659: [ValueTracking] peek through shuffles in ComputeNumSignBits (PR37549)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:2515
+  case Instruction::ShuffleVector:
+    // Look through shuffle of 1 source vector.
+    if (isa<UndefValue>(U->getOperand(0)))
----------------
efriedma wrote:
> spatel wrote:
> > RKSimon wrote:
> > > Do we know for certain that we don't reference an under operand at this point? SelectionDAG::ComputeNumSignBits bails i such cases.
> > I saw that, but I wasn't sure why bailing was necessary (in IR at least). 
> > 
> > If the shuffle's mask contains an undef, then the result for that lane is undef:
> > http://llvm.org/docs/LangRef.html#shufflevector-instruction
> > 
> > So we can choose 0 or -1 for that undef element, and so it can't reduce the number of sign bits?
> We can pick any value for an undef, but every optimization has to handle it consistently.  So you can't assume it's 0 unless you actually replace it with zero.
> 
> In this particular case, instead of looking for UndefValues, could you examine the mask instead?
IIUC, there are 2 independent factors here:
1. We need to examine the mask for undef values and bail out if it contains any.
2. If we look at both vector inputs to the shuffle, then we must take the minimum signbits value of those 2 values (we already do that in the DAG version). A potential complication for this enhancement is that I'm not sure if we handle an undef input here. That's why I made that case a TODO in this patch.


https://reviews.llvm.org/D53659