[PATCH] D66687: [x86] try to form more bt/test + set out of shift+mask patterns

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5225
+  //       compare in the source type.
+  unsigned ShiftAmt = Srl.getConstantOperandVal(1);
+  unsigned VTBits = VT.getSizeInBits();
----------------
craig.topper wrote:
> Is it possible for the shift to be on a type larger than 64 bits and have an absurdly out of bounds shift amount that we haven't collapsed to undef yet such that this asserts? Or for that matter a 64 bit shift amount that's larger than 0xffffffff and not been folded yet. Since we truncate a uint64_t to unsigned here.
Given current limits, I don't think either of those scenarios are possible. IR types are capped well below 2^32:
http://llvm.org/docs/LangRef.html#integer-type

And we shouldn't create an overshift node in the 1st place:
SelectionDAG::simplifyShift() - https://github.com/llvm-mirror/llvm/blob/master/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L7120

If that's somehow violated, then the underlying assert in APInt should fire within SDValue::getConstantOperandVal().

But there's practically no cost for leaving the shift amount as uint64_t rather than unsigned, so I can change that.


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

https://reviews.llvm.org/D66687