[llvm] r350198 - [X86] Factor the core code out of LowerXALUO into a helper function. Use it in LowerBRCOND and LowerSELECT to avoid some duplicated code.
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 1 11:34:11 PST 2019
Author: ctopper
Date: Tue Jan 1 11:34:11 2019
New Revision: 350198
URL: http://llvm.org/viewvc/llvm-project?rev=350198&view=rev
Log:
[X86] Factor the core code out of LowerXALUO into a helper function. Use it in LowerBRCOND and LowerSELECT to avoid some duplicated code.
This makes it easier to keep the LowerBRCOND and LowerSELECT code in sync with LowerXALUO so they always pick the same operation for overflowing instructions.
This is inspired by the helper functions used by ARM and AArch64 for the same purpose.
The test change is because LowerSELECT was not in sync with LowerXALUO with regard to INC/DEC for SADDO/SSUBO.
Modified:
llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
llvm/trunk/test/CodeGen/X86/xaluo.ll
Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=350198&r1=350197&r2=350198&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Tue Jan 1 11:34:11 2019
@@ -19588,6 +19588,93 @@ SDValue X86TargetLowering::LowerSETCCCAR
return getSETCC(CC, Cmp.getValue(1), DL, DAG);
}
+// This function returns three things: the arithmetic computation itself
+// (Value), an EFLAGS result (Overflow), and a condition code (Cond). The
+// flag and the condition code define the case in which the arithmetic
+// computation overflows.
+static std::pair<SDValue, SDValue>
+getX86XALUOOp(X86::CondCode &Cond, SDValue Op, SelectionDAG &DAG) {
+ assert(Op.getResNo() == 0 && "Unexpected result number!");
+ SDValue Value, Overflow;
+ SDValue LHS = Op.getOperand(0);
+ SDValue RHS = Op.getOperand(1);
+ unsigned BaseOp = 0;
+ SDLoc DL(Op);
+ switch (Op.getOpcode()) {
+ default: llvm_unreachable("Unknown ovf instruction!");
+ case ISD::SADDO:
+ // A subtract of one will be selected as a INC. Note that INC doesn't
+ // set CF, so we can't do this for UADDO.
+ if (isOneConstant(RHS)) {
+ BaseOp = X86ISD::INC;
+ Cond = X86::COND_O;
+ break;
+ }
+ BaseOp = X86ISD::ADD;
+ Cond = X86::COND_O;
+ break;
+ case ISD::UADDO:
+ BaseOp = X86ISD::ADD;
+ Cond = X86::COND_B;
+ break;
+ case ISD::SSUBO:
+ // A subtract of one will be selected as a DEC. Note that DEC doesn't
+ // set CF, so we can't do this for USUBO.
+ if (isOneConstant(RHS)) {
+ BaseOp = X86ISD::DEC;
+ Cond = X86::COND_O;
+ break;
+ }
+ BaseOp = X86ISD::SUB;
+ Cond = X86::COND_O;
+ break;
+ case ISD::USUBO:
+ BaseOp = X86ISD::SUB;
+ Cond = X86::COND_B;
+ break;
+ case ISD::SMULO:
+ BaseOp = Op.getValueType() == MVT::i8 ? X86ISD::SMUL8 : X86ISD::SMUL;
+ Cond = X86::COND_O;
+ break;
+ case ISD::UMULO: { // i64, i8 = umulo lhs, rhs --> i64, i64, i32 umul lhs,rhs
+ if (Op.getValueType() == MVT::i8) {
+ BaseOp = X86ISD::UMUL8;
+ Cond = X86::COND_O;
+ break;
+ }
+ SDVTList VTs = DAG.getVTList(Op.getValueType(), Op.getValueType(),
+ MVT::i32);
+ Value = DAG.getNode(X86ISD::UMUL, DL, VTs, LHS, RHS);
+ Overflow = Value.getValue(2);
+ Cond = X86::COND_O;
+ break;
+ }
+ }
+
+ if (BaseOp) {
+ // Also sets EFLAGS.
+ SDVTList VTs = DAG.getVTList(Op.getValueType(), MVT::i32);
+ Value = DAG.getNode(BaseOp, DL, VTs, LHS, RHS);
+ Overflow = Value.getValue(1);
+ }
+
+ return std::make_pair(Value, Overflow);
+}
+
+static SDValue LowerXALUO(SDValue Op, SelectionDAG &DAG) {
+ // Lower the "add/sub/mul with overflow" instruction into a regular ins plus
+ // a "setcc" instruction that checks the overflow flag. The "brcond" lowering
+ // looks for this combo and may remove the "setcc" instruction if the "setcc"
+ // has only one use.
+ SDLoc DL(Op);
+ X86::CondCode Cond;
+ SDValue Value, Overflow;
+ std::tie(Value, Overflow) = getX86XALUOOp(Cond, Op, DAG);
+
+ SDValue SetCC = getSETCC(Cond, Overflow, DL, DAG);
+ return DAG.getNode(ISD::MERGE_VALUES, DL, Op->getVTList(), Value, SetCC);
+}
+
/// Return true if opcode is a X86 logical comparison.
static bool isX86LogicalCmp(SDValue Op) {
unsigned Opc = Op.getOpcode();
@@ -19850,32 +19937,9 @@ SDValue X86TargetLowering::LowerSELECT(S
CondOpcode == ISD::UADDO || CondOpcode == ISD::SADDO ||
((CondOpcode == ISD::UMULO || CondOpcode == ISD::SMULO) &&
Cond.getOperand(0).getValueType() != MVT::i8)) {
- SDValue LHS = Cond.getOperand(0);
- SDValue RHS = Cond.getOperand(1);
- unsigned X86Opcode;
- unsigned X86Cond;
- SDVTList VTs;
- switch (CondOpcode) {
- case ISD::UADDO: X86Opcode = X86ISD::ADD; X86Cond = X86::COND_B; break;
- case ISD::SADDO: X86Opcode = X86ISD::ADD; X86Cond = X86::COND_O; break;
- case ISD::USUBO: X86Opcode = X86ISD::SUB; X86Cond = X86::COND_B; break;
- case ISD::SSUBO: X86Opcode = X86ISD::SUB; X86Cond = X86::COND_O; break;
- case ISD::UMULO: X86Opcode = X86ISD::UMUL; X86Cond = X86::COND_O; break;
- case ISD::SMULO: X86Opcode = X86ISD::SMUL; X86Cond = X86::COND_O; break;
- default: llvm_unreachable("unexpected overflowing operator");
- }
- if (CondOpcode == ISD::UMULO)
- VTs = DAG.getVTList(LHS.getValueType(), LHS.getValueType(),
- MVT::i32);
- else
- VTs = DAG.getVTList(LHS.getValueType(), MVT::i32);
-
- SDValue X86Op = DAG.getNode(X86Opcode, DL, VTs, LHS, RHS);
-
- if (CondOpcode == ISD::UMULO)
- Cond = X86Op.getValue(2);
- else
- Cond = X86Op.getValue(1);
+ SDValue Value;
+ X86::CondCode X86Cond;
+ std::tie(Value, Cond) = getX86XALUOOp(X86Cond, Cond.getValue(0), DAG);
CC = DAG.getConstant(X86Cond, DL, MVT::i8);
AddTest = false;
@@ -20557,47 +20621,12 @@ SDValue X86TargetLowering::LowerBRCOND(S
CondOpcode == ISD::USUBO || CondOpcode == ISD::SSUBO ||
((CondOpcode == ISD::UMULO || CondOpcode == ISD::SMULO) &&
Cond.getOperand(0).getValueType() != MVT::i8)) {
- SDValue LHS = Cond.getOperand(0);
- SDValue RHS = Cond.getOperand(1);
- unsigned X86Opcode;
- unsigned X86Cond;
- SDVTList VTs;
- // Keep this in sync with LowerXALUO, otherwise we might create redundant
- // instructions that can't be removed afterwards (i.e. X86ISD::ADD and
- // X86ISD::INC).
- switch (CondOpcode) {
- case ISD::UADDO: X86Opcode = X86ISD::ADD; X86Cond = X86::COND_B; break;
- case ISD::SADDO:
- if (isOneConstant(RHS)) {
- X86Opcode = X86ISD::INC; X86Cond = X86::COND_O;
- break;
- }
- X86Opcode = X86ISD::ADD; X86Cond = X86::COND_O; break;
- case ISD::USUBO: X86Opcode = X86ISD::SUB; X86Cond = X86::COND_B; break;
- case ISD::SSUBO:
- if (isOneConstant(RHS)) {
- X86Opcode = X86ISD::DEC; X86Cond = X86::COND_O;
- break;
- }
- X86Opcode = X86ISD::SUB; X86Cond = X86::COND_O; break;
- case ISD::UMULO: X86Opcode = X86ISD::UMUL; X86Cond = X86::COND_O; break;
- case ISD::SMULO: X86Opcode = X86ISD::SMUL; X86Cond = X86::COND_O; break;
- default: llvm_unreachable("unexpected overflowing operator");
- }
- if (Inverted)
- X86Cond = X86::GetOppositeBranchCondition((X86::CondCode)X86Cond);
- if (CondOpcode == ISD::UMULO)
- VTs = DAG.getVTList(LHS.getValueType(), LHS.getValueType(),
- MVT::i32);
- else
- VTs = DAG.getVTList(LHS.getValueType(), MVT::i32);
-
- SDValue X86Op = DAG.getNode(X86Opcode, dl, VTs, LHS, RHS);
+ SDValue Value;
+ X86::CondCode X86Cond;
+ std::tie(Value, Cond) = getX86XALUOOp(X86Cond, Cond.getValue(0), DAG);
- if (CondOpcode == ISD::UMULO)
- Cond = X86Op.getValue(2);
- else
- Cond = X86Op.getValue(1);
+ if (Inverted)
+ X86Cond = X86::GetOppositeBranchCondition(X86Cond);
CC = DAG.getConstant(X86Cond, dl, MVT::i8);
addTest = false;
@@ -24920,78 +24949,6 @@ static SDValue LowerRotate(SDValue Op, c
DAG.getVectorShuffle(VT, DL, Res02, Res13, {1, 5, 3, 7}));
}
-static SDValue LowerXALUO(SDValue Op, SelectionDAG &DAG) {
- // Lower the "add/sub/mul with overflow" instruction into a regular ins plus
- // a "setcc" instruction that checks the overflow flag. The "brcond" lowering
- // looks for this combo and may remove the "setcc" instruction if the "setcc"
- // has only one use.
- SDNode *N = Op.getNode();
- SDValue LHS = N->getOperand(0);
- SDValue RHS = N->getOperand(1);
- unsigned BaseOp = 0;
- X86::CondCode Cond;
- SDLoc DL(Op);
- switch (Op.getOpcode()) {
- default: llvm_unreachable("Unknown ovf instruction!");
- case ISD::SADDO:
- // A subtract of one will be selected as a INC. Note that INC doesn't
- // set CF, so we can't do this for UADDO.
- if (isOneConstant(RHS)) {
- BaseOp = X86ISD::INC;
- Cond = X86::COND_O;
- break;
- }
- BaseOp = X86ISD::ADD;
- Cond = X86::COND_O;
- break;
- case ISD::UADDO:
- BaseOp = X86ISD::ADD;
- Cond = X86::COND_B;
- break;
- case ISD::SSUBO:
- // A subtract of one will be selected as a DEC. Note that DEC doesn't
- // set CF, so we can't do this for USUBO.
- if (isOneConstant(RHS)) {
- BaseOp = X86ISD::DEC;
- Cond = X86::COND_O;
- break;
- }
- BaseOp = X86ISD::SUB;
- Cond = X86::COND_O;
- break;
- case ISD::USUBO:
- BaseOp = X86ISD::SUB;
- Cond = X86::COND_B;
- break;
- case ISD::SMULO:
- BaseOp = N->getValueType(0) == MVT::i8 ? X86ISD::SMUL8 : X86ISD::SMUL;
- Cond = X86::COND_O;
- break;
- case ISD::UMULO: { // i64, i8 = umulo lhs, rhs --> i64, i64, i32 umul lhs,rhs
- if (N->getValueType(0) == MVT::i8) {
- BaseOp = X86ISD::UMUL8;
- Cond = X86::COND_O;
- break;
- }
- SDVTList VTs = DAG.getVTList(N->getValueType(0), N->getValueType(0),
- MVT::i32);
- SDValue Sum = DAG.getNode(X86ISD::UMUL, DL, VTs, LHS, RHS);
-
- SDValue SetCC = getSETCC(X86::COND_O, SDValue(Sum.getNode(), 2), DL, DAG);
-
- return DAG.getNode(ISD::MERGE_VALUES, DL, N->getVTList(), Sum, SetCC);
- }
- }
-
- // Also sets EFLAGS.
- SDVTList VTs = DAG.getVTList(N->getValueType(0), MVT::i32);
- SDValue Sum = DAG.getNode(BaseOp, DL, VTs, LHS, RHS);
-
- SDValue SetCC = getSETCC(Cond, SDValue(Sum.getNode(), 1), DL, DAG);
-
- return DAG.getNode(ISD::MERGE_VALUES, DL, N->getVTList(), Sum, SetCC);
-}
-
/// Returns true if the operand type is exactly twice the native width, and
/// the corresponding cmpxchg8b or cmpxchg16b instruction is available.
/// Used to know whether to use cmpxchg8/16b when expanding atomic operations
Modified: llvm/trunk/test/CodeGen/X86/xaluo.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/xaluo.ll?rev=350198&r1=350197&r2=350198&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/xaluo.ll (original)
+++ llvm/trunk/test/CodeGen/X86/xaluo.ll Tue Jan 1 11:34:11 2019
@@ -1078,15 +1078,13 @@ define {i64, i1} @usuboovf(i64 %a, i64 %
ret {i64, i1} %t
}
-; FIXME: We're selecting both an INC and an ADD here. One of them becomes an LEA.
+; Make sure we select an INC for both the data use and the flag use.
define i32 @incovfselectstore(i32 %v1, i32 %v2, i32* %x) {
; SDAG-LABEL: incovfselectstore:
; SDAG: ## %bb.0:
; SDAG-NEXT: movl %esi, %eax
-; SDAG-NEXT: ## kill: def $edi killed $edi def $rdi
-; SDAG-NEXT: leal 1(%rdi), %ecx
-; SDAG-NEXT: movl %edi, %esi
-; SDAG-NEXT: addl $1, %esi
+; SDAG-NEXT: movl %edi, %ecx
+; SDAG-NEXT: incl %ecx
; SDAG-NEXT: cmovol %edi, %eax
; SDAG-NEXT: movl %ecx, (%rdx)
; SDAG-NEXT: retq
@@ -1107,15 +1105,13 @@ define i32 @incovfselectstore(i32 %v1, i
ret i32 %ret
}
-; FIXME: We're selecting both a DEC and a SUB here. DEC becomes an LEA and
-; SUB becomes a CMP.
+; Make sure we select a DEC for both the data use and the flag use.
define i32 @decovfselectstore(i32 %v1, i32 %v2, i32* %x) {
; SDAG-LABEL: decovfselectstore:
; SDAG: ## %bb.0:
; SDAG-NEXT: movl %esi, %eax
-; SDAG-NEXT: ## kill: def $edi killed $edi def $rdi
-; SDAG-NEXT: leal -1(%rdi), %ecx
-; SDAG-NEXT: cmpl $1, %edi
+; SDAG-NEXT: movl %edi, %ecx
+; SDAG-NEXT: decl %ecx
; SDAG-NEXT: cmovol %edi, %eax
; SDAG-NEXT: movl %ecx, (%rdx)
; SDAG-NEXT: retq
More information about the llvm-commits
mailing list