[llvm] cda6b09 - [X86] Make sure we do not clobber RBX with mwaitx when used as a base
Pierre Gousseau via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 26 03:20:48 PDT 2020
Author: Pierre Gousseau
Date: 2020-08-26T11:20:31+01:00
New Revision: cda6b0924257f162cc9299dae2d4bb134fac5d38
URL: https://github.com/llvm/llvm-project/commit/cda6b0924257f162cc9299dae2d4bb134fac5d38
DIFF: https://github.com/llvm/llvm-project/commit/cda6b0924257f162cc9299dae2d4bb134fac5d38.diff
LOG: [X86] Make sure we do not clobber RBX with mwaitx when used as a base
pointer.
mwaitx uses EBX as one of its argument.
Using this instruction clobbers RBX as it is defined to hold one of the
input. When the backend uses dynamically allocated stack, RBX is used as
a reserved register for the base pointer.
This patch is adapted from @qcolombet patch for cmpxchg at r263325.
This fixes PR43528.
Reviewed By: craig.topper
Differential Revision: https://reviews.llvm.org/D73475
Added:
llvm/test/CodeGen/X86/base-pointer-and-mwaitx.ll
Modified:
llvm/lib/Target/X86/X86ExpandPseudo.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
llvm/lib/Target/X86/X86InstrCompiler.td
llvm/lib/Target/X86/X86InstrInfo.td
Removed:
################################################################################
diff --git a/llvm/lib/Target/X86/X86ExpandPseudo.cpp b/llvm/lib/Target/X86/X86ExpandPseudo.cpp
index c47ef4708e91..a07e165633bb 100644
--- a/llvm/lib/Target/X86/X86ExpandPseudo.cpp
+++ b/llvm/lib/Target/X86/X86ExpandPseudo.cpp
@@ -442,6 +442,29 @@ bool X86ExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
MBB.erase(MBBI);
return true;
}
+ case X86::MWAITX_SAVE_EBX:
+ case X86::MWAITX_SAVE_RBX: {
+ // Perform the following transformation.
+ // SaveRbx = pseudomwaitx InArg, SaveRbx
+ // =>
+ // [E|R]BX = InArg
+ // actualmwaitx
+ // [E|R]BX = SaveRbx
+ const MachineOperand &InArg = MBBI->getOperand(1);
+ // Copy the input argument of the pseudo into the argument of the
+ // actual instruction.
+ TII->copyPhysReg(MBB, MBBI, DL, X86::EBX, InArg.getReg(), InArg.isKill());
+ // Create the actual instruction.
+ BuildMI(MBB, MBBI, DL, TII->get(X86::MWAITXrrr));
+ // Finally, restore the value of RBX.
+ Register SaveRbx = MBBI->getOperand(2).getReg();
+ unsigned BasePointer = Opcode == X86::MWAITX_SAVE_EBX ? X86::EBX : X86::RBX;
+ TII->copyPhysReg(MBB, MBBI, DL, BasePointer, SaveRbx,
+ /*SrcIsKill*/ true);
+ // Delete the pseudo.
+ MBBI->eraseFromParent();
+ return true;
+ }
case TargetOpcode::ICALL_BRANCH_FUNNEL:
ExpandICallBranchFunnel(&MBB, MBBI);
return true;
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index e203689e64ee..7440f3238448 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -25817,6 +25817,20 @@ static SDValue LowerINTRINSIC_W_CHAIN(SDValue Op, const X86Subtarget &Subtarget,
return DAG.getNode(ISD::MERGE_VALUES, dl, Op->getVTList(), SetCC,
Operation.getValue(1));
}
+ case Intrinsic::x86_mwaitx: {
+ // If the current function needs the base pointer, RBX,
+ // we shouldn't use mwaitx directly.
+ // Indeed the lowering of that instruction will clobber
+ // that register and since RBX will be a reserved register
+ // the register allocator will not make sure its value will
+ // be properly saved and restored around this live-range.
+ SDLoc dl(Op);
+ unsigned Opcode = X86ISD::MWAITX_DAG;
+ SDValue Chain = DAG.getNode(Opcode, dl, MVT::Other,
+ {Op->getOperand(0), Op->getOperand(2),
+ Op->getOperand(3), Op->getOperand(4)});
+ return Chain;
+ }
}
return SDValue();
}
@@ -30538,6 +30552,7 @@ const char *X86TargetLowering::getTargetNodeName(unsigned Opcode) const {
NODE_NAME_CASE(LCMPXCHG16_DAG)
NODE_NAME_CASE(LCMPXCHG8_SAVE_EBX_DAG)
NODE_NAME_CASE(LCMPXCHG16_SAVE_RBX_DAG)
+ NODE_NAME_CASE(MWAITX_DAG)
NODE_NAME_CASE(LADD)
NODE_NAME_CASE(LSUB)
NODE_NAME_CASE(LOR)
@@ -33497,6 +33512,48 @@ X86TargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
BB->addLiveIn(BasePtr);
return BB;
}
+ case X86::MWAITX: {
+ const X86RegisterInfo *TRI = Subtarget.getRegisterInfo();
+ Register BasePtr = TRI->getBaseRegister();
+ bool IsRBX = (BasePtr == X86::RBX || BasePtr == X86::EBX);
+ // If no need to save the base pointer, we generate MWAITXrrr,
+ // else we generate pseudo MWAITX_SAVE_RBX/EBX.
+ if (!IsRBX || !TRI->hasBasePointer(*MF)) {
+ BuildMI(*BB, MI, DL, TII->get(TargetOpcode::COPY), X86::ECX)
+ .addReg(MI.getOperand(0).getReg());
+ BuildMI(*BB, MI, DL, TII->get(TargetOpcode::COPY), X86::EAX)
+ .addReg(MI.getOperand(1).getReg());
+ BuildMI(*BB, MI, DL, TII->get(TargetOpcode::COPY), X86::EBX)
+ .addReg(MI.getOperand(2).getReg());
+ BuildMI(*BB, MI, DL, TII->get(X86::MWAITXrrr));
+ MI.eraseFromParent();
+ } else {
+ if (!BB->isLiveIn(BasePtr)) {
+ BB->addLiveIn(BasePtr);
+ }
+ // Parameters can be copied into ECX and EAX but not EBX yet.
+ BuildMI(*BB, MI, DL, TII->get(TargetOpcode::COPY), X86::ECX)
+ .addReg(MI.getOperand(0).getReg());
+ BuildMI(*BB, MI, DL, TII->get(TargetOpcode::COPY), X86::EAX)
+ .addReg(MI.getOperand(1).getReg());
+ const TargetRegisterClass *RegClass =
+ BasePtr == X86::EBX ? &X86::GR32RegClass : &X86::GR64RegClass;
+ // Save RBX (or EBX) into a virtual register.
+ Register SaveRBX = MF->getRegInfo().createVirtualRegister(RegClass);
+ BuildMI(*BB, MI, DL, TII->get(TargetOpcode::COPY), SaveRBX)
+ .addReg(BasePtr);
+ // Generate mwaitx pseudo.
+ unsigned Opcode =
+ BasePtr == X86::RBX ? X86::MWAITX_SAVE_RBX : X86::MWAITX_SAVE_EBX;
+ Register Dst = MF->getRegInfo().createVirtualRegister(RegClass);
+ BuildMI(*BB, MI, DL, TII->get(Opcode))
+ .addDef(Dst) // Destination tied in with SaveRBX.
+ .addReg(MI.getOperand(2).getReg()) // input value of EBX.
+ .addUse(SaveRBX); // Save of base pointer.
+ MI.eraseFromParent();
+ }
+ return BB;
+ }
case TargetOpcode::PREALLOCATED_SETUP: {
assert(Subtarget.is32Bit() && "preallocated only used in 32-bit");
auto MFI = MF->getInfo<X86MachineFunctionInfo>();
diff --git a/llvm/lib/Target/X86/X86ISelLowering.h b/llvm/lib/Target/X86/X86ISelLowering.h
index 2a870b53dc82..7c977ce9e3fa 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.h
+++ b/llvm/lib/Target/X86/X86ISelLowering.h
@@ -749,6 +749,9 @@ namespace llvm {
STRICT_CVTPS2PH,
STRICT_CVTPH2PS,
+ // Mwaitx builtin is lowered to this if the base pointer needs saving.
+ MWAITX_DAG,
+
// Compare and swap.
LCMPXCHG_DAG = ISD::FIRST_TARGET_MEMORY_OPCODE,
LCMPXCHG8_DAG,
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index d78d9f7c80c7..4f81c271386c 100644
--- a/llvm/lib/Target/X86/X86InstrCompiler.td
+++ b/llvm/lib/Target/X86/X86InstrCompiler.td
@@ -896,6 +896,44 @@ def LCMPXCHG16B_SAVE_RBX :
GR64:$rbx_save))]>;
}
+// This pseudo must be used when the frame uses RBX as
+// the base pointer.
+// cf comment for LCMPXCHG8B_SAVE_EBX.
+let Defs = [ECX, EAX, EBX, EFLAGS], Uses = [ECX, EAX, EBX],
+ Predicates = [HasMWAITX], SchedRW = [WriteSystem],
+ isCodeGenOnly = 1, isPseudo = 1, Constraints = "$ebx_save = $dst",
+ usesCustomInserter = 1 in {
+def MWAITX_SAVE_EBX :
+ I<0, Pseudo, (outs GR32:$dst),
+ (ins GR32:$ebx_input, GR32:$ebx_save),
+ "mwaitx",
+ []>;
+}
+// Same as MWAITX_SAVE_EBX but for the case where RBX is the base pointer.
+let Defs = [ECX, EAX, EBX, EFLAGS], Uses = [ECX, EAX, EBX],
+ Predicates = [HasMWAITX], SchedRW = [WriteSystem],
+ isCodeGenOnly = 1, isPseudo = 1, Constraints = "$rbx_save = $dst",
+ usesCustomInserter = 1 in {
+def MWAITX_SAVE_RBX :
+ I<0, Pseudo, (outs GR64:$dst),
+ (ins GR32:$ebx_input, GR64:$rbx_save),
+ "mwaitx",
+ []>;
+}
+
+// Pseudo mwaitx instruction to use for custom insertion.
+let Defs = [ECX, EAX, EBX, EFLAGS], Uses = [ECX, EAX, EBX],
+ Predicates = [HasMWAITX], SchedRW = [WriteSystem],
+ isCodeGenOnly = 1, isPseudo = 1,
+ usesCustomInserter = 1 in {
+def MWAITX :
+ I<0, Pseudo, (outs),
+ (ins GR32:$ecx, GR32:$eax, GR32:$ebx),
+ "mwaitx",
+ [(X86mwaitx GR32:$ecx, GR32:$eax, GR32:$ebx)]>;
+}
+
+
defm LCMPXCHG : LCMPXCHG_BinOp<0xB0, 0xB1, MRMDestMem, "cmpxchg", X86cas>;
// Atomic exchange and add
diff --git a/llvm/lib/Target/X86/X86InstrInfo.td b/llvm/lib/Target/X86/X86InstrInfo.td
index 8d2178066d4f..14ca9f889e17 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.td
+++ b/llvm/lib/Target/X86/X86InstrInfo.td
@@ -77,6 +77,9 @@ def SDTX86caspairSaveRbx16 : SDTypeProfile<1, 3,
[SDTCisVT<0, i64>, SDTCisPtrTy<1>,
SDTCisVT<2, i64>, SDTCisVT<3, i64>]>;
+def SDTX86mwaitx : SDTypeProfile<0, 3, [SDTCisVT<0, i32>, SDTCisVT<1, i32>,
+ SDTCisVT<2, i32>]>;
+
def SDTLockBinaryArithWithFlags : SDTypeProfile<1, 2, [SDTCisVT<0, i32>,
SDTCisPtrTy<1>,
SDTCisInt<2>]>;
@@ -184,6 +187,10 @@ def X86cas16save_rbx : SDNode<"X86ISD::LCMPXCHG16_SAVE_RBX_DAG",
[SDNPHasChain, SDNPInGlue, SDNPOutGlue,
SDNPMayStore, SDNPMayLoad, SDNPMemOperand]>;
+def X86mwaitx : SDNode<"X86ISD::MWAITX_DAG", SDTX86mwaitx,
+ [SDNPHasChain, SDNPInGlue, SDNPOutGlue, SDNPMayStore,
+ SDNPMayLoad]>;
+
def X86retflag : SDNode<"X86ISD::RET_FLAG", SDTX86Ret,
[SDNPHasChain, SDNPOptInGlue, SDNPVariadic]>;
def X86iret : SDNode<"X86ISD::IRET", SDTX86Ret,
diff --git a/llvm/test/CodeGen/X86/base-pointer-and-mwaitx.ll b/llvm/test/CodeGen/X86/base-pointer-and-mwaitx.ll
new file mode 100644
index 000000000000..fead4c650c0a
--- /dev/null
+++ b/llvm/test/CodeGen/X86/base-pointer-and-mwaitx.ll
@@ -0,0 +1,210 @@
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -mattr=+mwaitx -x86-use-base-pointer=true -stackrealign -stack-alignment=32 %s -o - | FileCheck --check-prefix=CHECK --check-prefix=USE_BASE_64 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnux32 -mattr=+mwaitx -x86-use-base-pointer=true -stackrealign -stack-alignment=32 %s -o - | FileCheck --check-prefix=CHECK --check-prefix=USE_BASE_32 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -mattr=+mwaitx -x86-use-base-pointer=true %s -o - | FileCheck --check-prefix=CHECK --check-prefix=NO_BASE_64 %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnux32 -mattr=+mwaitx -x86-use-base-pointer=true %s -o - | FileCheck --check-prefix=CHECK --check-prefix=NO_BASE_32 %s
+
+; This test checks that we save and restore the base pointer (ebx or rbx) in the
+; presence of the mwaitx intrinsic which requires to use ebx for one of its
+; argument.
+; This function uses a dynamically allocated stack to force the use
+; of a base pointer.
+; After the call to the mwaitx intrinsic we do a volatile store to the
+; dynamically allocated memory which will require the use of the base pointer.
+; The base pointer should therefore be restored straight after the mwaitx
+; instruction.
+
+define void @test_baseptr(i64 %x, i64 %y, i32 %E, i32 %H, i32 %C) nounwind {
+entry:
+ %ptr = alloca i8*, align 8
+ %0 = alloca i8, i64 %x, align 16
+ store i8* %0, i8** %ptr, align 8
+ call void @llvm.x86.mwaitx(i32 %E, i32 %H, i32 %C)
+ %1 = load i8*, i8** %ptr, align 8
+ %arrayidx = getelementptr inbounds i8, i8* %1, i64 %y
+ store volatile i8 42, i8* %arrayidx, align 1
+ ret void
+}
+; CHECK-LABEL: test_baseptr:
+; USE_BASE_64: movq %rsp, %rbx
+; Pass mwaitx first 2 arguments in eax and ecx respectively.
+; USE_BASE_64: movl %ecx, %eax
+; USE_BASE_64: movl %edx, %ecx
+; Save base pointer.
+; USE_BASE_64: movq %rbx, [[SAVE_rbx:%r([8-9]|1[0-5]|di|si)]]
+; Set mwaitx ebx argument.
+; USE_BASE_64: movl %r8d, %ebx
+; USE_BASE_64-NEXT: mwaitx
+; Restore base pointer.
+; USE_BASE_64-NEXT: movq [[SAVE_rbx]], %rbx
+
+; USE_BASE_32: movl %esp, %ebx
+; Pass mwaitx first 2 arguments in eax and ecx respectively.
+; USE_BASE_32: movl %ecx, %eax
+; USE_BASE_32: movl %edx, %ecx
+; Save base pointer.
+; USE_BASE_32: movl %ebx, [[SAVE_ebx:%e(di|si)]]
+; Set mwaitx ebx argument.
+; USE_BASE_32: movl %r8d, %ebx
+; USE_BASE_32-NEXT: mwaitx
+; Restore base pointer.
+; USE_BASE_32-NEXT: movl [[SAVE_ebx]], %ebx
+
+; Pass mwaitx 3 arguments in eax, ecx, ebx
+; NO_BASE_64: movl %r8d, %ebx
+; NO_BASE_64: movl %ecx, %eax
+; NO_BASE_64: movl %edx, %ecx
+; No need to save base pointer.
+; NO_BASE_64-NOT: movq %rbx
+; NO_BASE_64: mwaitx
+; No need to restore base pointer.
+; NO_BASE_64-NOT: movq {{.*}}, %rbx
+; NO_BASE_64-NEXT: {{.+$}}
+
+; Pass mwaitx 3 arguments in eax, ecx, ebx
+; NO_BASE_32: movl %r8d, %ebx
+; NO_BASE_32: movl %ecx, %eax
+; NO_BASE_32: movl %edx, %ecx
+; No need to save base pointer.
+; NO_BASE_32-NOT: movl %ebx
+; NO_BASE_32: mwaitx
+; No need to restore base pointer.
+; NO_BASE_32-NOT: movl {{.*}}, %ebx
+; NO_BASE_32-NEXT: {{.+$}}
+
+; Test of the case where an opaque sp adjustement is introduced by a separate
+; basic block which, combined with stack realignment, requires a base pointer.
+ at g = global i32 0, align 8
+
+define void @test_opaque_sp_adjustment(i32 %E, i32 %H, i32 %C, i64 %x) {
+entry:
+ %ptr = alloca i8*, align 8
+ call void @llvm.x86.mwaitx(i32 %E, i32 %H, i32 %C)
+ %g = load i32, i32* @g, align 4
+ %tobool = icmp ne i32 %g, 0
+ br i1 %tobool, label %if.then, label %if.end
+
+if.then:
+ call void asm sideeffect "", "~{rsp},~{esp},~{dirflag},~{fpsr},~{flags}"()
+ br label %if.end
+
+if.end:
+ %ptr2 = load i8*, i8** %ptr, align 8
+ %arrayidx = getelementptr inbounds i8, i8* %ptr2, i64 %x
+ store volatile i8 42, i8* %arrayidx, align 1
+ ret void
+}
+; CHECK-LABEL: test_opaque_sp_adjustment:
+; USE_BASE_64: movq %rsp, %rbx
+; Pass mwaitx first 2 arguments in eax and ecx respectively.
+; USE_BASE_64: movl %esi, %eax
+; USE_BASE_64: movl %edi, %ecx
+; Save base pointer.
+; USE_BASE_64: movq %rbx, [[SAVE_rbx:%r([8-9]|1[0-5]|di|si)]]
+; Set mwaitx ebx argument.
+; USE_BASE_64: movl %edx, %ebx
+; USE_BASE_64-NEXT: mwaitx
+; Restore base pointer.
+; USE_BASE_64-NEXT: movq [[SAVE_rbx]], %rbx
+
+; USE_BASE_32: movl %esp, %ebx
+; Pass mwaitx first 2 arguments in eax and ecx respectively.
+; USE_BASE_32: movl %esi, %eax
+; USE_BASE_32: movl %edi, %ecx
+; Save base pointer.
+; USE_BASE_32: movl %ebx, [[SAVE_ebx:%e(di|si)]]
+; Set mwaitx ebx argument.
+; USE_BASE_32: movl %edx, %ebx
+; USE_BASE_32-NEXT: mwaitx
+; Restore base pointer.
+; USE_BASE_32-NEXT: movl [[SAVE_ebx]], %ebx
+
+; Pass mwaitx 3 arguments in eax, ecx, ebx
+; NO_BASE_64: movl %edx, %ebx
+; NO_BASE_64: movl %esi, %eax
+; NO_BASE_64: movl %edi, %ecx
+; No need to save base pointer.
+; NO_BASE_64-NOT: movq %rbx
+; NO_BASE_64: mwaitx
+; NO_BASE_64-NOT: movq {{.*}}, %rbx
+; NO_BASE_64-NEXT: {{.+$}}
+
+; Pass mwaitx 3 arguments in eax, ecx, ebx
+; NO_BASE_32: movl %edx, %ebx
+; NO_BASE_32: movl %esi, %eax
+; NO_BASE_32: movl %edi, %ecx
+; No need to save base pointer.
+; NO_BASE_32-NOT: movl %ebx
+; NO_BASE_32: mwaitx
+; No need to restore base pointer.
+; NO_BASE_32-NOT: movl {{.*}}, %ebx
+; NO_BASE_32-NEXT: {{.+$}}
+
+; Test of the case where a variable size object is introduced by a separate
+; basic block which, combined with stack realignment, requires a base pointer.
+define void @test_variable_size_object(i32 %E, i32 %H, i32 %C, i64 %x) {
+entry:
+ %ptr = alloca i8*, align 8
+ call void @llvm.x86.mwaitx(i32 %E, i32 %H, i32 %C)
+ %g = load i32, i32* @g, align 4
+ %tobool = icmp ne i32 %g, 0
+ br i1 %tobool, label %if.then, label %if.end
+
+if.then:
+ %i5 = alloca i8, i64 %x, align 16
+ store i8* %i5, i8** %ptr, align 8
+ br label %if.end
+
+if.end:
+ %ptr2 = load i8*, i8** %ptr, align 8
+ %arrayidx = getelementptr inbounds i8, i8* %ptr2, i64 %x
+ store volatile i8 42, i8* %arrayidx, align 1
+ ret void
+}
+
+; CHECK-LABEL: test_variable_size_object:
+; USE_BASE_64: movq %rsp, %rbx
+; Pass mwaitx first 2 arguments in eax and ecx respectively.
+; USE_BASE_64: movl %esi, %eax
+; USE_BASE_64: movl %edi, %ecx
+; Save base pointer.
+; USE_BASE_64: movq %rbx, [[SAVE_rbx:%r([8-9]|1[0-5]|di|si)]]
+; Set mwaitx ebx argument.
+; USE_BASE_64: movl %edx, %ebx
+; USE_BASE_64-NEXT: mwaitx
+; Restore base pointer.
+; USE_BASE_64-NEXT: movq [[SAVE_rbx]], %rbx
+
+; USE_BASE_32: movl %esp, %ebx
+; Pass mwaitx first 2 arguments in eax and ecx respectively.
+; USE_BASE_32: movl %esi, %eax
+; USE_BASE_32: movl %edi, %ecx
+; Save base pointer.
+; USE_BASE_32: movl %ebx, [[SAVE_ebx:%e(di|si)]]
+; Set mwaitx ebx argument.
+; USE_BASE_32: movl %edx, %ebx
+; USE_BASE_32-NEXT: mwaitx
+; Restore base pointer.
+; USE_BASE_32-NEXT: movl [[SAVE_ebx]], %ebx
+
+; Pass mwaitx 3 arguments in eax, ecx, ebx
+; NO_BASE_64: movl %edx, %ebx
+; NO_BASE_64: movl %esi, %eax
+; NO_BASE_64: movl %edi, %ecx
+; No need to save base pointer.
+; NO_BASE_64-NOT: movq %rbx
+; NO_BASE_64: mwaitx
+; NO_BASE_64-NOT: movq {{.*}}, %rbx
+; NO_BASE_64-NEXT: {{.+$}}
+
+; Pass mwaitx 3 arguments in eax, ecx, ebx
+; NO_BASE_32: movl %edx, %ebx
+; NO_BASE_32: movl %esi, %eax
+; NO_BASE_32: movl %edi, %ecx
+; No need to save base pointer.
+; NO_BASE_32-NOT: movl %ebx
+; NO_BASE_32: mwaitx
+; No need to restore base pointer.
+; NO_BASE_32-NOT: movl {{.*}}, %ebx
+; NO_BASE_32-NEXT: {{.+$}}
+
+declare void @llvm.x86.mwaitx(i32, i32, i32) nounwind
More information about the llvm-commits
mailing list