[SelectionDAG] Fix incorrect condition for shift amount truncation

[nextsilicon-itay-bookstein]

Summary

Comment says:

// If the operand is larger than the shift count type but the shift
// count type has enough bits to represent any shift value ...

It clearly talks about the shifted operand, not the shift-amount operand,
but the comparison is performed against Log2_32_Ceil(Op2.getValueSizeInBits())
where Op2 is the shift amount operand. This comparison also doesn't make
sense in the context of the previous one (ShiftsSize > Op2Size) because
Op2Size == Op2.getValueSizeInBits(). Fix to use Op1.

[craig.topper]

I think Op1 and Op2 are the same size at this point in the code due to the requirements of the IR operation. But this is more readable.

LGTM

[nextsilicon-itay-bookstein]

True, at SelectionDAGBuilder time that invariant holds. So just speedbump removal / potential copypasta bug prevention? 😛

I have no commit access, might I ask that you commit this on my behalf?

Author: Itay Bookstein <[e-mail]>

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (!I.getType()->isVectorTy() && Op2.getValueType() != ShiftTy) {
     // If the operand is smaller than the shift count type, promote it.
     if (ShiftSize > Op2Size)
       Op2 = DAG.getNode(ISD::ZERO_EXTEND, DL, ShiftTy, Op2);
 
     // If the operand is larger than the shift count type but the shift
     // count type has enough bits to represent any shift value, truncate
     // it now. This is a common case and it exposes the truncate to
     // optimization early.
-    else if (ShiftSize >= Log2_32_Ceil(Op2.getValueSizeInBits()))
+    else if (ShiftSize >= Log2_32_Ceil(Op1.getValueSizeInBits()))
       Op2 = DAG.getNode(ISD::TRUNCATE, DL, ShiftTy, Op2);
     // Otherwise we'll need to temporarily settle for some other convenient
     // type.  Type legalization will make adjustments once the shiftee is split.
     else
       Op2 = DAG.getZExtOrTrunc(Op2, DL, MVT::i32);
   }
 
   bool nuw = false;