[llvm-commits] [PATCH] add-carray/sub-borrow optimization (CodeGen, X86)
Shuxin Yang
shuxin.llvm at gmail.com
Wed Oct 31 13:51:55 PDT 2012
Hi, Manman:
You are right! Please find the revised patch in the attachment.
In this revised patch, I factor out the non-trivial code and put it
into a tiny function called MaterializeSETB().
Now we can return early by calling this tiny function, and the dead code
is deleted as well.
Thanks
Shuxin
On 10/31/12 10:37 AM, manman ren wrote:
> On Oct 31, 2012, at 10:30 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>
>> Hi, Manman:
>>
>> I read "coding guide" the other day. The document also suggests "return early" to make the logic easier.
>> But it doesn't say on what condition the "return early" is appropriate. In this case, we have to duplicate
>> non-trivial code in order to return early. For sloppy engineers like me, maintaining duplication is difficult.
>> I'd like to improve maintenance even at the cost of sacrificed readability.
> We can probably get rid of
>>>>> + SDValue NewVal = DAG.getNode(X86ISD::SETCC, DL, N->getVTList(),
>>>>> + DAG.getConstant(CC, MVT::i8), EFLAGS);
>>>>> + N = NewVal.getNode ();
> since SETCC will be replaced with a SETCC_CARRAY and N is not used in the followed if statement.
>
> Otherwise LGTM.
>
> Thanks,
>
>
-------------- next part --------------
Index: test/CodeGen/X86/jump_sign.ll
===================================================================
--- test/CodeGen/X86/jump_sign.ll (revision 167148)
+++ test/CodeGen/X86/jump_sign.ll (working copy)
@@ -219,7 +219,6 @@
; by sbb, we should not optimize cmp away.
define i32 @q(i32 %j.4, i32 %w, i32 %el) {
; CHECK: q:
-; CHECK: sub
; CHECK: cmp
; CHECK-NEXT: sbb
%tmp532 = add i32 %j.4, %w
Index: test/CodeGen/X86/add-of-carry.ll
===================================================================
--- test/CodeGen/X86/add-of-carry.ll (revision 167148)
+++ test/CodeGen/X86/add-of-carry.ll (working copy)
@@ -30,4 +30,17 @@
ret i32 %z.0
}
+; <rdar://problem/12579915>
+define i32 @test3(i32 %x, i32 %y, i32 %res) nounwind uwtable readnone ssp {
+entry:
+ %cmp = icmp ugt i32 %x, %y
+ %dec = sext i1 %cmp to i32
+ %dec.res = add nsw i32 %dec, %res
+ ret i32 %dec.res
+; CHECK: test3:
+; CHECK: cmpl
+; CHECK: sbbl
+; CHECK: ret
+}
+
declare { i32, i1 } @llvm.uadd.with.overflow.i32(i32, i32) nounwind readnone
Index: lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- lib/Target/X86/X86ISelLowering.cpp (revision 167148)
+++ lib/Target/X86/X86ISelLowering.cpp (working copy)
@@ -16476,6 +16476,16 @@
return SDValue();
}
+// Helper function of PerformSETCCCombine. It is to materialize "setb reg"
+// as "sbb reg,reg", since it can be extended without zext and produces
+// an all-ones bit which is more useful than 0/1 in some cases.
+static SDValue MaterializeSETB(DebugLoc DL, SDValue EFLAGS, SelectionDAG &DAG) {
+ return DAG.getNode(ISD::AND, DL, MVT::i8,
+ DAG.getNode(X86ISD::SETCC_CARRY, DL, MVT::i8,
+ DAG.getConstant(X86::COND_B, MVT::i8), EFLAGS),
+ DAG.getConstant(1, MVT::i8));
+}
+
// Optimize RES = X86ISD::SETCC CONDCODE, EFLAG_INPUT
static SDValue PerformSETCCCombine(SDNode *N, SelectionDAG &DAG,
TargetLowering::DAGCombinerInfo &DCI,
@@ -16484,14 +16494,29 @@
X86::CondCode CC = X86::CondCode(N->getConstantOperandVal(0));
SDValue EFLAGS = N->getOperand(1);
+ if (CC == X86::COND_A) {
+ // Try to convert COND_A into COND_B in an attempt to facilitate
+ // materializing "setb reg".
+ //
+ // Do not flip "e > c", where "c" is a constant, because Cmp instruction
+ // cannot take an immediate as its first operand.
+ //
+ if (EFLAGS.getOpcode() == X86ISD::SUB && EFLAGS.hasOneUse() &&
+ EFLAGS.getValueType().isInteger() &&
+ !isa<ConstantSDNode>(EFLAGS.getOperand(1))) {
+ SDValue NewSub = DAG.getNode(X86ISD::SUB, EFLAGS.getDebugLoc(),
+ EFLAGS.getNode()->getVTList(),
+ EFLAGS.getOperand(1), EFLAGS.getOperand(0));
+ SDValue NewEFLAGS = SDValue(NewSub.getNode(), EFLAGS.getResNo());
+ return MaterializeSETB(DL, NewEFLAGS, DAG);
+ }
+ }
+
// Materialize "setb reg" as "sbb reg,reg", since it can be extended without
// a zext and produces an all-ones bit which is more useful than 0/1 in some
// cases.
if (CC == X86::COND_B)
- return DAG.getNode(ISD::AND, DL, MVT::i8,
- DAG.getNode(X86ISD::SETCC_CARRY, DL, MVT::i8,
- DAG.getConstant(CC, MVT::i8), EFLAGS),
- DAG.getConstant(1, MVT::i8));
+ return MaterializeSETB(DL, EFLAGS, DAG);
SDValue Flags;
More information about the llvm-commits
mailing list