[PATCH] D67021: [DAGCombiner] improve throughput of shift+logic+shift

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7240-7243
+    // Shift amount types do not have to match their operand type, so check that
+    // the constants are the same width.
+    if (ShiftAmtVal->getBitWidth() != C1Val.getBitWidth())
+      return false;
----------------
spatel wrote:
> lebedev.ri wrote:
> > This looks suspicious to be honest.
> > Is there a case where that is so?
> > If not, can this be an assert until then?
> It's real - SDAG is full of surprising situations like this. I hit an assert in a regression test without this.
> 
> That was before I added the !LegalTypes constraint, so it might be hidden now, but there are few bounds down here.
getShiftAmountTy gets used in most cases when a combine creates a new shift, so it is likely to have occurred before legalization.


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

https://reviews.llvm.org/D67021