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

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7217-7219
+  if (LogicOpcode != ISD::AND && LogicOpcode != ISD::OR &&
+      LogicOpcode != ISD::XOR)
+    return SDValue();
----------------
lebedev.ri wrote:
> Is there some function already that has whitelist of such bitwise ops?
> I suspect there are more candidates.
Target-specific opcodes? And/or/xor are the only generic opcodes that I am aware of.


================
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;
----------------
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.


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

https://reviews.llvm.org/D67021