[PATCH] D30502: [DAGCombiner] fold binops with constant into select-of-constants
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 1 11:07:21 PST 2017
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
More information about the llvm-commits
mailing list