From 0ff51d5dde29dfd9bc5064e32d47345bab7a5034 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Fri, 10 Jun 2022 13:37:49 -0700 Subject: [PATCH] Fix interaction of CFI instructions with MachineOutliner. 1. When checking if a candidate contains a CFI instruction, actually iterate over all of the instructions, instead of stopping halfway through. 2. Make sure copied CFI directives refer to the correct instruction. Fixes https://github.com/llvm/llvm-project/issues/55842 Differential Revision: https://reviews.llvm.org/D126930 --- llvm/lib/CodeGen/MachineOutliner.cpp | 19 ++--- llvm/lib/CodeGen/MachineVerifier.cpp | 6 ++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 8 +-- llvm/lib/Target/X86/X86InstrInfo.cpp | 8 +-- llvm/test/CodeGen/AArch64/machine-outliner-cfi.mir | 81 ++++++++++++++++------ 5 files changed, 84 insertions(+), 38 deletions(-) diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp index 7ab6996..5da68ab 100644 --- a/llvm/lib/CodeGen/MachineOutliner.cpp +++ b/llvm/lib/CodeGen/MachineOutliner.cpp @@ -665,17 +665,20 @@ MachineFunction *MachineOutliner::createOutlinedFunction( ++I) { if (I->isDebugInstr()) continue; - MachineInstr *NewMI = MF.CloneMachineInstr(&*I); + + // Don't keep debug information for outlined instructions. + auto DL = DebugLoc(); if (I->isCFIInstruction()) { - unsigned CFIIndex = NewMI->getOperand(0).getCFIIndex(); + unsigned CFIIndex = I->getOperand(0).getCFIIndex(); MCCFIInstruction CFI = Instrs[CFIIndex]; - (void)MF.addFrameInst(CFI); + BuildMI(MBB, MBB.end(), DL, TII.get(TargetOpcode::CFI_INSTRUCTION)) + .addCFIIndex(MF.addFrameInst(CFI)); + } else { + MachineInstr *NewMI = MF.CloneMachineInstr(&*I); + NewMI->dropMemRefs(MF); + NewMI->setDebugLoc(DL); + MBB.insert(MBB.end(), NewMI); } - NewMI->dropMemRefs(MF); - - // Don't keep debug information for outlined instructions. - NewMI->setDebugLoc(DebugLoc()); - MBB.insert(MBB.end(), NewMI); } // Set normal properties for a late MachineFunction. diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index 084f858..db04f2b 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -64,6 +64,7 @@ #include "llvm/InitializePasses.h" #include "llvm/MC/LaneBitmask.h" #include "llvm/MC/MCAsmInfo.h" +#include "llvm/MC/MCDwarf.h" #include "llvm/MC/MCInstrDesc.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCTargetOptions.h" @@ -2212,6 +2213,11 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) { } break; + case MachineOperand::MO_CFIIndex: + if (MO->getCFIIndex() >= MF->getFrameInstructions().size()) + report("CFI instruction has invalid index", MO, MONum); + break; + default: break; } diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 9f647d6..b093001 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -7012,12 +7012,10 @@ outliner::OutlinedFunction AArch64InstrInfo::getOutliningCandidateInfo( // We check to see if CFI Instructions are present, and if they are // we find the number of CFI Instructions in the candidates. unsigned CFICount = 0; - MachineBasicBlock::iterator MBBI = RepeatedSequenceLocs[0].front(); - for (unsigned Loc = RepeatedSequenceLocs[0].getStartIdx(); - Loc < RepeatedSequenceLocs[0].getEndIdx() + 1; Loc++) { - if (MBBI->isCFIInstruction()) + for (auto &I : make_range(RepeatedSequenceLocs[0].front(), + std::next(RepeatedSequenceLocs[0].back()))) { + if (I.isCFIInstruction()) CFICount++; - MBBI++; } // We compare the number of found CFI Instructions to the number of CFI diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 6da6cb2..e20c738 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -9487,12 +9487,10 @@ outliner::OutlinedFunction X86InstrInfo::getOutliningCandidateInfo( // We check to see if CFI Instructions are present, and if they are // we find the number of CFI Instructions in the candidates. unsigned CFICount = 0; - MachineBasicBlock::iterator MBBI = RepeatedSequenceLocs[0].front(); - for (unsigned Loc = RepeatedSequenceLocs[0].getStartIdx(); - Loc < RepeatedSequenceLocs[0].getEndIdx() + 1; Loc++) { - if (MBBI->isCFIInstruction()) + for (auto &I : make_range(RepeatedSequenceLocs[0].front(), + std::next(RepeatedSequenceLocs[0].back()))) { + if (I.isCFIInstruction()) CFICount++; - MBBI++; } // We compare the number of found CFI Instructions to the number of CFI diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-cfi.mir b/llvm/test/CodeGen/AArch64/machine-outliner-cfi.mir index 5d98646..a44a90e 100644 --- a/llvm/test/CodeGen/AArch64/machine-outliner-cfi.mir +++ b/llvm/test/CodeGen/AArch64/machine-outliner-cfi.mir @@ -1,15 +1,17 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py -# RUN: llc -mtriple=aarch64-apple-unknown -run-pass=machine-outliner -verify-machineinstrs %s -o - | FileCheck %s +# RUN: llc -mtriple=aarch64-apple-unknown -run-pass=machine-outliner -verify-machineinstrs -enable-machine-outliner=always %s -o - | FileCheck %s # Outlining CFI instructions is unsafe if it is not tail called, but otherwise, # it requires fixups. Show that we don't include CFI instructions in non -# tail call outlined sequences right now. +# tail call outlined sequences right now. Show that we count CFI instructions +# correctly in the presence of debug info. --- | define void @foo() #0 { ret void } define void @bar() #0 { ret void } define void @baz() #0 { ret void } attributes #0 = { noredzone } + ... --- name: foo @@ -19,15 +21,28 @@ body: | liveins: $lr ; CHECK-LABEL: name: foo ; CHECK: liveins: $lr - ; CHECK: $w9 = ORRWri $wzr, 1 - ; CHECK: $w10 = ORRWri $wzr, 2 - ; CHECK: $w11 = ORRWri $wzr, 3 - ; CHECK: frame-setup CFI_INSTRUCTION def_cfa $w29, 16 - ; CHECK: $w20 = ORRWri $wzr, 1 - ; CHECK: TCRETURNdi @OUTLINED_FUNCTION_0, 0, implicit $sp, implicit-def $w12, implicit-def $w13, implicit-def $w14, implicit-def $w15, implicit $wzr, implicit $sp + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $w9 = ORRWri $wzr, 1 + ; CHECK-NEXT: $w10 = ORRWri $wzr, 2 + ; CHECK-NEXT: $w11 = ORRWri $wzr, 3 + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa $w29, 16 + ; CHECK-NEXT: $w20 = ORRWri $wzr, 1 + ; CHECK-NEXT: TCRETURNdi @OUTLINED_FUNCTION_0, 0, implicit $sp, implicit-def $w12, implicit-def $w13, implicit-def $w14, implicit-def $w15, implicit $wzr, implicit $sp $w9 = ORRWri $wzr, 1 $w10 = ORRWri $wzr, 2 $w11 = ORRWri $wzr, 3 + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg frame-setup CFI_INSTRUCTION def_cfa $w29, 16 $w20 = ORRWri $wzr, 1 $w12 = ORRWri $wzr, 1 @@ -44,15 +59,28 @@ body: | liveins: $lr ; CHECK-LABEL: name: bar ; CHECK: liveins: $lr - ; CHECK: $w9 = ORRWri $wzr, 1 - ; CHECK: $w10 = ORRWri $wzr, 2 - ; CHECK: $w11 = ORRWri $wzr, 3 - ; CHECK: frame-setup CFI_INSTRUCTION def_cfa $w29, 16 - ; CHECK: $w21 = ORRWri $wzr, 1 - ; CHECK: TCRETURNdi @OUTLINED_FUNCTION_0, 0, implicit $sp, implicit-def $w12, implicit-def $w13, implicit-def $w14, implicit-def $w15, implicit $wzr, implicit $sp + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $w9 = ORRWri $wzr, 1 + ; CHECK-NEXT: $w10 = ORRWri $wzr, 2 + ; CHECK-NEXT: $w11 = ORRWri $wzr, 3 + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa $w29, 16 + ; CHECK-NEXT: $w21 = ORRWri $wzr, 1 + ; CHECK-NEXT: TCRETURNdi @OUTLINED_FUNCTION_0, 0, implicit $sp, implicit-def $w12, implicit-def $w13, implicit-def $w14, implicit-def $w15, implicit $wzr, implicit $sp $w9 = ORRWri $wzr, 1 $w10 = ORRWri $wzr, 2 $w11 = ORRWri $wzr, 3 + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg frame-setup CFI_INSTRUCTION def_cfa $w29, 16 $w21 = ORRWri $wzr, 1 $w12 = ORRWri $wzr, 1 @@ -69,15 +97,28 @@ body: | liveins: $lr ; CHECK-LABEL: name: baz ; CHECK: liveins: $lr - ; CHECK: $w9 = ORRWri $wzr, 1 - ; CHECK: $w10 = ORRWri $wzr, 2 - ; CHECK: $w11 = ORRWri $wzr, 3 - ; CHECK: frame-setup CFI_INSTRUCTION def_cfa $w29, 16 - ; CHECK: $w22 = ORRWri $wzr, 1 - ; CHECK: TCRETURNdi @OUTLINED_FUNCTION_0, 0, implicit $sp, implicit-def $w12, implicit-def $w13, implicit-def $w14, implicit-def $w15, implicit $wzr, implicit $sp + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $w9 = ORRWri $wzr, 1 + ; CHECK-NEXT: $w10 = ORRWri $wzr, 2 + ; CHECK-NEXT: $w11 = ORRWri $wzr, 3 + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: DBG_VALUE $w9, $noreg + ; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa $w29, 16 + ; CHECK-NEXT: $w22 = ORRWri $wzr, 1 + ; CHECK-NEXT: TCRETURNdi @OUTLINED_FUNCTION_0, 0, implicit $sp, implicit-def $w12, implicit-def $w13, implicit-def $w14, implicit-def $w15, implicit $wzr, implicit $sp $w9 = ORRWri $wzr, 1 $w10 = ORRWri $wzr, 2 $w11 = ORRWri $wzr, 3 + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg + DBG_VALUE $w9, $noreg frame-setup CFI_INSTRUCTION def_cfa $w29, 16 $w22 = ORRWri $wzr, 1 $w12 = ORRWri $wzr, 1 -- 2.7.4