[PATCH] D30502: [DAGCombiner] fold binops with constant into select-of-constants

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> The debug output for "sel_constants_shl_constant" makes me think that this patch is not at fault for that mess.

I would agree with that assessment; you get the same terrible lowering for "select i1 %cond, i8 -20, i8 115".

---

Some targets intentionally fold the other way in certain cases: pulling a math operation out of a select can make the immediates cheaper.  For example, consider "int y; int *a(int x) { return x ? &y : &y+1; }" on ARM.  That isn't directly impacted by this, though, because ARM uses target-specific nodes.

Does it make sense to handle SELECT_CC in addition to SELECT?



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1788
+  SDValue NewCT = DAG.getNode(BinOpcode, DL, VT, CT, C1);
+  SDValue NewCF = DAG.getNode(BinOpcode, DL, VT, CF, C1);
+  return DAG.getSelect(DL, VT, Sel.getOperand(0), NewCT, NewCF);
----------------
Could you assert here that NewCT and NewCF are actually constants?


================
Comment at: test/CodeGen/X86/setcc.ll:54
+; CHECK-NEXT:    adcw $0, %ax
+; CHECK-NEXT:    shll $16, %eax
 ; CHECK-NEXT:    retq
----------------
Hmm... we really want movl+adcl rather than movw+adcw.  Maybe orthogonal to this patch.


https://reviews.llvm.org/D30502