From c822bf16138bf54ba230d11e2e2ea1a9c9147494 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Mon, 14 Nov 2016 19:02:17 +0000 Subject: [PATCH] ARM: sort register lists by encoding in push/pop instructions. For example we were producing push {r8, r10, r11, r4, r5, r7, lr} This is misleading (r4, r5 and r7 are actually pushed before the rest), and other components (stack folding recently) often forget to deal with the extra complexity coming from the different order, leading to miscompiles. Finally, we warn about our own code in -no-integrated-as mode without this, which is really not a good idea. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286866 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/ARMFrameLowering.cpp | 15 ++++++++++++++- lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp | 6 ++++++ lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp | 9 ++++++++- test/CodeGen/ARM/swifterror.ll | 8 ++++---- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/lib/Target/ARM/ARMFrameLowering.cpp b/lib/Target/ARM/ARMFrameLowering.cpp index 29f4c099f47..ac0b55c419d 100644 --- a/lib/Target/ARM/ARMFrameLowering.cpp +++ b/lib/Target/ARM/ARMFrameLowering.cpp @@ -893,10 +893,12 @@ void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB, unsigned MIFlags) const { MachineFunction &MF = *MBB.getParent(); const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo(); + const TargetRegisterInfo &TRI = *STI.getRegisterInfo(); DebugLoc DL; - SmallVector, 4> Regs; + typedef std::pair RegAndKill; + SmallVector Regs; unsigned i = CSI.size(); while (i != 0) { unsigned LastReg = 0; @@ -927,6 +929,11 @@ void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB, if (Regs.empty()) continue; + + std::sort(Regs.begin(), Regs.end(), [&](RegAndKill &LHS, RegAndKill &RHS) { + return TRI.getEncodingValue(LHS.first) < TRI.getEncodingValue(RHS.first); + }); + if (Regs.size() > 1 || StrOpc== 0) { MachineInstrBuilder MIB = AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(StmOpc), ARM::SP) @@ -960,6 +967,7 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB, unsigned NumAlignedDPRCS2Regs) const { MachineFunction &MF = *MBB.getParent(); const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo(); + const TargetRegisterInfo &TRI = *STI.getRegisterInfo(); ARMFunctionInfo *AFI = MF.getInfo(); DebugLoc DL; bool isTailCall = false; @@ -1012,6 +1020,11 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB, if (Regs.empty()) continue; + + std::sort(Regs.begin(), Regs.end(), [&](unsigned &LHS, unsigned &RHS) { + return TRI.getEncodingValue(LHS) < TRI.getEncodingValue(RHS); + }); + if (Regs.size() > 1 || LdrOpc == 0) { MachineInstrBuilder MIB = AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(LdmOpc), ARM::SP) diff --git a/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp b/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp index e81bb77dbdf..3667952d44c 100644 --- a/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp +++ b/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp @@ -726,6 +726,12 @@ void ARMInstPrinter::printPKHASRShiftImm(const MCInst *MI, unsigned OpNum, void ARMInstPrinter::printRegisterList(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { + assert(std::is_sorted(MI->begin() + OpNum, MI->end(), + [&](const MCOperand &LHS, const MCOperand &RHS) { + return MRI.getEncodingValue(LHS.getReg()) < + MRI.getEncodingValue(RHS.getReg()); + })); + O << "{"; for (unsigned i = OpNum, e = MI->getNumOperands(); i != e; ++i) { if (i != OpNum) diff --git a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp index 093e2b545a4..559a4f8de75 100644 --- a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp +++ b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp @@ -1545,8 +1545,15 @@ getRegisterListOpValue(const MCInst &MI, unsigned Op, else Binary |= NumRegs * 2; } else { + const MCRegisterInfo &MRI = *CTX.getRegisterInfo(); + assert(std::is_sorted(MI.begin() + Op, MI.end(), + [&](const MCOperand &LHS, const MCOperand &RHS) { + return MRI.getEncodingValue(LHS.getReg()) < + MRI.getEncodingValue(RHS.getReg()); + })); + for (unsigned I = Op, E = MI.getNumOperands(); I < E; ++I) { - unsigned RegNo = CTX.getRegisterInfo()->getEncodingValue(MI.getOperand(I).getReg()); + unsigned RegNo = MRI.getEncodingValue(MI.getOperand(I).getReg()); Binary |= 1 << RegNo; } } diff --git a/test/CodeGen/ARM/swifterror.ll b/test/CodeGen/ARM/swifterror.ll index a3539537d72..7551291207e 100644 --- a/test/CodeGen/ARM/swifterror.ll +++ b/test/CodeGen/ARM/swifterror.ll @@ -415,7 +415,7 @@ define swiftcc void @swifterror_reg_clobber(%swift_error** nocapture %err) { ; CHECK-ARMV7-LABEL: _params_in_reg ; Store callee saved registers excluding swifterror. -; CHECK-ARMV7: push {r8, r10, r11, r4, r5, r7, lr} +; CHECK-ARMV7: push {r4, r5, r7, r8, r10, r11, lr} ; Store swiftself (r10) and swifterror (r6). ; CHECK-ARMV7-DAG: str r6, [s[[STK1:.*]]] ; CHECK-ARMV7-DAG: str r10, [s[[STK2:.*]]] @@ -440,7 +440,7 @@ define swiftcc void @swifterror_reg_clobber(%swift_error** nocapture %err) { ; CHECK-ARMV7: mov r2, r5 ; CHECK-ARMV7: mov r3, r4 ; CHECK-ARMV7: bl _params_in_reg2 -; CHECK-ARMV7: pop {r8, r10, r11, r4, r5, r7, pc} +; CHECK-ARMV7: pop {r4, r5, r7, r8, r10, r11, pc} define swiftcc void @params_in_reg(i32, i32, i32, i32, i8* swiftself, %swift_error** nocapture swifterror %err) { %error_ptr_ref = alloca swifterror %swift_error*, align 8 store %swift_error* null, %swift_error** %error_ptr_ref @@ -451,7 +451,7 @@ define swiftcc void @params_in_reg(i32, i32, i32, i32, i8* swiftself, %swift_err declare swiftcc void @params_in_reg2(i32, i32, i32, i32, i8* swiftself, %swift_error** nocapture swifterror %err) ; CHECK-ARMV7-LABEL: params_and_return_in_reg -; CHECK-ARMV7: push {r8, r10, r11, r4, r5, r7, lr} +; CHECK-ARMV7: push {r4, r5, r7, r8, r10, r11, lr} ; Store swifterror and swiftself ; CHECK-ARMV7: mov r4, r6 ; CHECK-ARMV7: str r10, [s[[STK1:.*]]] @@ -502,7 +502,7 @@ declare swiftcc void @params_in_reg2(i32, i32, i32, i32, i8* swiftself, %swift_e ; CHECK-ARMV7: mov r1, r4 ; CHECK-ARMV7: mov r2, r8 ; CHECK-ARMV7: mov r3, r11 -; CHECK-ARMV7: pop {r8, r10, r11, r4, r5, r7, pc} +; CHECK-ARMV7: pop {r4, r5, r7, r8, r10, r11, pc} define swiftcc { i32, i32, i32, i32} @params_and_return_in_reg(i32, i32, i32, i32, i8* swiftself, %swift_error** nocapture swifterror %err) { %error_ptr_ref = alloca swifterror %swift_error*, align 8 store %swift_error* null, %swift_error** %error_ptr_ref -- 2.11.0