From b25820f47f6580f3a9d8028d30f5abff12720999 Mon Sep 17 00:00:00 2001 From: WANG Rui Date: Thu, 16 Oct 2025 13:44:25 +0800 Subject: [PATCH 1/2] [LoongArch] Add `isSafeToMove` hook to prevent unsafe instruction motion This patch introduces a new virtual method `TargetInstrInfo::isSafeToMove()` to allow backends to control whether a machine instruction can be safely moved by optimization passes. The `BranchFolder` pass now respects this hook when hoisting common code. By default, all instructions are considered safe to to move. For LoongArch, `isSafeToMove()` is overridden to prevent relocation-related instruction sequences (e.g. PC-relative addressing and calls) from being broken by instruction motion. Correspondingly, `isSchedulingBoundary()` is updated to reuse this logic for consistency. --- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 11 +++++ llvm/lib/CodeGen/BranchFolding.cpp | 9 +++- llvm/lib/CodeGen/BranchFolding.h | 2 +- .../Target/LoongArch/LoongArchInstrInfo.cpp | 45 +++++++++++-------- .../lib/Target/LoongArch/LoongArchInstrInfo.h | 3 ++ 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index 175f205328361..2dcedfb40f3e6 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -1761,6 +1761,17 @@ class LLVM_ABI TargetInstrInfo : public MCInstrInfo { return true; } + /// Return true if it's safe to move a machine instruction. + /// This allows the backend to prevent certain special instruction + /// sequences from being broken by instruction motion in optimization + /// passes. + /// By default, this returns true for every instruction. + virtual bool isSafeToMove(const MachineInstr &MI, + const MachineBasicBlock *MBB, + const MachineFunction &MF) const { + return true; + } + /// Test if the given instruction should be considered a scheduling boundary. /// This primarily includes labels and terminators. virtual bool isSchedulingBoundary(const MachineInstr &MI, diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 7292bc2be0df2..8213a91a7750d 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -1818,7 +1818,7 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) { bool BranchFolder::HoistCommonCode(MachineFunction &MF) { bool MadeChange = false; for (MachineBasicBlock &MBB : llvm::make_early_inc_range(MF)) - MadeChange |= HoistCommonCodeInSuccs(&MBB); + MadeChange |= HoistCommonCodeInSuccs(&MBB, MF); return MadeChange; } @@ -1948,7 +1948,8 @@ MachineBasicBlock::iterator findHoistingInsertPosAndDeps(MachineBasicBlock *MBB, return PI; } -bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { +bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB, + MachineFunction &MF) { MachineBasicBlock *TBB = nullptr, *FBB = nullptr; SmallVector Cond; if (TII->analyzeBranch(*MBB, TBB, FBB, Cond, true) || !TBB || Cond.empty()) @@ -1993,6 +1994,10 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { // Hard to reason about register liveness with predicated instruction. break; + if (!TII->isSafeToMove(*TIB, MBB, MF)) + // Don't hoist the instruction if it isn't safe to move. + break; + bool IsSafe = true; for (MachineOperand &MO : TIB->operands()) { // Don't attempt to hoist instructions with register masks. diff --git a/llvm/lib/CodeGen/BranchFolding.h b/llvm/lib/CodeGen/BranchFolding.h index ff2bbe06c0488..8f0f14367e0eb 100644 --- a/llvm/lib/CodeGen/BranchFolding.h +++ b/llvm/lib/CodeGen/BranchFolding.h @@ -196,7 +196,7 @@ class TargetRegisterInfo; /// If the successors of MBB has common instruction sequence at the start of /// the function, move the instructions before MBB terminator if it's legal. - bool HoistCommonCodeInSuccs(MachineBasicBlock *MBB); + bool HoistCommonCodeInSuccs(MachineBasicBlock *MBB, MachineFunction &MF); }; } // end namespace llvm diff --git a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp index c89212dae72d9..2b792184087cd 100644 --- a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp +++ b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp @@ -378,12 +378,9 @@ bool LoongArchInstrInfo::isBranchOffsetInRange(unsigned BranchOp, } } -bool LoongArchInstrInfo::isSchedulingBoundary(const MachineInstr &MI, - const MachineBasicBlock *MBB, - const MachineFunction &MF) const { - if (TargetInstrInfo::isSchedulingBoundary(MI, MBB, MF)) - return true; - +bool LoongArchInstrInfo::isSafeToMove(const MachineInstr &MI, + const MachineBasicBlock *MBB, + const MachineFunction &MF) const { auto MII = MI.getIterator(); auto MIE = MBB->end(); @@ -429,25 +426,25 @@ bool LoongArchInstrInfo::isSchedulingBoundary(const MachineInstr &MI, auto MO2 = Lu32I->getOperand(2).getTargetFlags(); if (MO0 == LoongArchII::MO_PCREL_HI && MO1 == LoongArchII::MO_PCREL_LO && MO2 == LoongArchII::MO_PCREL64_LO) - return true; + return false; if ((MO0 == LoongArchII::MO_GOT_PC_HI || MO0 == LoongArchII::MO_LD_PC_HI || MO0 == LoongArchII::MO_GD_PC_HI) && MO1 == LoongArchII::MO_GOT_PC_LO && MO2 == LoongArchII::MO_GOT_PC64_LO) - return true; + return false; if (MO0 == LoongArchII::MO_IE_PC_HI && MO1 == LoongArchII::MO_IE_PC_LO && MO2 == LoongArchII::MO_IE_PC64_LO) - return true; + return false; if (MO0 == LoongArchII::MO_DESC_PC_HI && MO1 == LoongArchII::MO_DESC_PC_LO && MO2 == LoongArchII::MO_DESC64_PC_LO) - return true; + return false; break; } case LoongArch::LU52I_D: { auto MO = MI.getOperand(2).getTargetFlags(); if (MO == LoongArchII::MO_PCREL64_HI || MO == LoongArchII::MO_GOT_PC64_HI || MO == LoongArchII::MO_IE_PC64_HI || MO == LoongArchII::MO_DESC64_PC_HI) - return true; + return false; break; } default: @@ -487,7 +484,7 @@ bool LoongArchInstrInfo::isSchedulingBoundary(const MachineInstr &MI, auto MO1 = LoongArchII::getDirectFlags(SecondOp->getOperand(2)); auto MO2 = LoongArchII::getDirectFlags(Ld->getOperand(2)); if (MO1 == LoongArchII::MO_DESC_PC_LO && MO2 == LoongArchII::MO_DESC_LD) - return true; + return false; break; } if (SecondOp == MIE || @@ -496,34 +493,34 @@ bool LoongArchInstrInfo::isSchedulingBoundary(const MachineInstr &MI, auto MO1 = LoongArchII::getDirectFlags(SecondOp->getOperand(2)); if (MO0 == LoongArchII::MO_PCREL_HI && SecondOp->getOpcode() == AddiOp && MO1 == LoongArchII::MO_PCREL_LO) - return true; + return false; if (MO0 == LoongArchII::MO_GOT_PC_HI && SecondOp->getOpcode() == LdOp && MO1 == LoongArchII::MO_GOT_PC_LO) - return true; + return false; if ((MO0 == LoongArchII::MO_LD_PC_HI || MO0 == LoongArchII::MO_GD_PC_HI) && SecondOp->getOpcode() == AddiOp && MO1 == LoongArchII::MO_GOT_PC_LO) - return true; + return false; break; } case LoongArch::ADDI_W: case LoongArch::ADDI_D: { auto MO = LoongArchII::getDirectFlags(MI.getOperand(2)); if (MO == LoongArchII::MO_PCREL_LO || MO == LoongArchII::MO_GOT_PC_LO) - return true; + return false; break; } case LoongArch::LD_W: case LoongArch::LD_D: { auto MO = LoongArchII::getDirectFlags(MI.getOperand(2)); if (MO == LoongArchII::MO_GOT_PC_LO) - return true; + return false; break; } case LoongArch::PseudoDESC_CALL: { auto MO = LoongArchII::getDirectFlags(MI.getOperand(2)); if (MO == LoongArchII::MO_DESC_CALL) - return true; + return false; break; } default: @@ -531,6 +528,18 @@ bool LoongArchInstrInfo::isSchedulingBoundary(const MachineInstr &MI, } } + return true; +} + +bool LoongArchInstrInfo::isSchedulingBoundary(const MachineInstr &MI, + const MachineBasicBlock *MBB, + const MachineFunction &MF) const { + if (TargetInstrInfo::isSchedulingBoundary(MI, MBB, MF)) + return true; + + if (!isSafeToMove(MI, MBB, MF)) + return true; + return false; } diff --git a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.h b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.h index f25958a32bec4..8457d1446ed5e 100644 --- a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.h +++ b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.h @@ -64,6 +64,9 @@ class LoongArchInstrInfo : public LoongArchGenInstrInfo { bool isBranchOffsetInRange(unsigned BranchOpc, int64_t BrOffset) const override; + bool isSafeToMove(const MachineInstr &MI, const MachineBasicBlock *MBB, + const MachineFunction &MF) const override; + bool isSchedulingBoundary(const MachineInstr &MI, const MachineBasicBlock *MBB, const MachineFunction &MF) const override; From 9ef66f30714d026c4ef65bb894e181b8c55ffe2f Mon Sep 17 00:00:00 2001 From: WANG Rui Date: Thu, 16 Oct 2025 21:32:36 +0800 Subject: [PATCH 2/2] Address zhaoqi's comments --- llvm/lib/CodeGen/BranchFolding.cpp | 6 +++--- llvm/lib/CodeGen/BranchFolding.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 8213a91a7750d..af1625a209569 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -1818,7 +1818,7 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) { bool BranchFolder::HoistCommonCode(MachineFunction &MF) { bool MadeChange = false; for (MachineBasicBlock &MBB : llvm::make_early_inc_range(MF)) - MadeChange |= HoistCommonCodeInSuccs(&MBB, MF); + MadeChange |= HoistCommonCodeInSuccs(&MBB); return MadeChange; } @@ -1948,8 +1948,7 @@ MachineBasicBlock::iterator findHoistingInsertPosAndDeps(MachineBasicBlock *MBB, return PI; } -bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB, - MachineFunction &MF) { +bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { MachineBasicBlock *TBB = nullptr, *FBB = nullptr; SmallVector Cond; if (TII->analyzeBranch(*MBB, TBB, FBB, Cond, true) || !TBB || Cond.empty()) @@ -1980,6 +1979,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB, MachineBasicBlock::iterator FIB = FBB->begin(); MachineBasicBlock::iterator TIE = TBB->end(); MachineBasicBlock::iterator FIE = FBB->end(); + MachineFunction &MF = *MBB->getParent(); while (TIB != TIE && FIB != FIE) { // Skip dbg_value instructions. These do not count. TIB = skipDebugInstructionsForward(TIB, TIE, false); diff --git a/llvm/lib/CodeGen/BranchFolding.h b/llvm/lib/CodeGen/BranchFolding.h index 8f0f14367e0eb..ff2bbe06c0488 100644 --- a/llvm/lib/CodeGen/BranchFolding.h +++ b/llvm/lib/CodeGen/BranchFolding.h @@ -196,7 +196,7 @@ class TargetRegisterInfo; /// If the successors of MBB has common instruction sequence at the start of /// the function, move the instructions before MBB terminator if it's legal. - bool HoistCommonCodeInSuccs(MachineBasicBlock *MBB, MachineFunction &MF); + bool HoistCommonCodeInSuccs(MachineBasicBlock *MBB); }; } // end namespace llvm