[RISCV] Add RemoveBackToBackBranches Pass

djtodoro · djtodoro · commit 4f3953147ab0 · 2024-11-27T12:54:15.000+01:00
According to the MIPS specification, there shouldn't be
two conditional branches in the same 8-byte aligned
region of code.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -4941,6 +4941,7 @@ def mload_store_pairs : Flag<["-"], "mload-store-pairs">, Group<m_riscv_Features
 def mno_load_store_pairs : Flag<["-"], "mno-load-store-pairs">, Group<m_riscv_Features_Group>;
 def mccmov : Flag<["-"], "mccmov">, Group<m_riscv_Features_Group>;
 def mno_ccmov : Flag<["-"], "mno-ccmov">, Group<m_riscv_Features_Group>;
+def mremove_back_to_back_branches : Flag<["-"], "mremove_back_to_back_branches">, Group<m_riscv_Features_Group>;
 let Flags = [TargetSpecific] in {
 def menable_experimental_extensions : Flag<["-"], "menable-experimental-extensions">, Group<m_Group>,
   HelpText<"Enable use of experimental RISC-V extensions.">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2178,6 +2178,10 @@ void Clang::AddRISCVTargetArgs(const ArgList &Args,
       CmdArgs.push_back("-riscv-ccmov=0");
     }
   }
+  if (Args.getLastArg(options::OPT_mremove_back_to_back_branches)) {
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back("-riscv-remove-back-to-back-branches=1");
+  }
   // Handle -mrvv-vector-bits=<bits>
   if (Arg *A = Args.getLastArg(options::OPT_mrvv_vector_bits_EQ)) {
     StringRef Val = A->getValue();
diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt
@@ -55,6 +55,7 @@ add_llvm_target(RISCVCodeGen
   RISCVMoveMerger.cpp
   RISCVPushPopOptimizer.cpp
   RISCVRegisterInfo.cpp
+  RISCVRemoveBackToBackBranches.cpp
   RISCVSubtarget.cpp
   RISCVTargetMachine.cpp
   RISCVTargetObjectFile.cpp
diff --git a/llvm/lib/Target/RISCV/RISCV.h b/llvm/lib/Target/RISCV/RISCV.h
@@ -96,6 +96,9 @@ createRISCVInstructionSelector(const RISCVTargetMachine &,
                                const RISCVRegisterBankInfo &);
 void initializeRISCVDAGToDAGISelLegacyPass(PassRegistry &);
 
+FunctionPass *createRISCVRemoveBackToBackBranches();
+void initializeRISCVRemoveBackToBackBranchesPass(PassRegistry &);
+
 FunctionPass *createRISCVPostLegalizerCombiner();
 void initializeRISCVPostLegalizerCombinerPass(PassRegistry &);
 
diff --git a/llvm/lib/Target/RISCV/RISCVRemoveBackToBackBranches.cpp b/llvm/lib/Target/RISCV/RISCVRemoveBackToBackBranches.cpp
@@ -0,0 +1,158 @@
+//===----------------------- RISCVRemoveBackToBackBranches.cpp ------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RISCV.h"
+#include "RISCVInstrInfo.h"
+#include "RISCVSubtarget.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/TargetSubtargetInfo.h"
+#include "llvm/Target/TargetMachine.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "riscv-remove-back-to-back-branches"
+
+STATISTIC(NumInsertedAligments, "Number of aligments set");
+
+namespace {
+
+// According to the MIPS specification, there shouldn't be two conditional
+// branches in the same 8-byte aligned region of code.
+constexpr unsigned NumberOfBytesOfCodeRegion = 8;
+
+class RISCVRemoveBackToBackBranches : public MachineFunctionPass {
+public:
+  static char ID;
+
+  RISCVRemoveBackToBackBranches() : MachineFunctionPass(ID) {
+    initializeRISCVRemoveBackToBackBranchesPass(
+        *PassRegistry::getPassRegistry());
+  }
+
+  StringRef getPassName() const override {
+    return "RISCV Remove Back To Back Branches Pass";
+  }
+
+  bool runOnMachineFunction(MachineFunction &F) override;
+
+  MachineFunctionProperties getRequiredProperties() const override {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::NoVRegs);
+  }
+
+private:
+  const RISCVSubtarget *STI;
+  const RISCVInstrInfo *TII;
+};
+
+} // end of anonymous namespace
+
+char RISCVRemoveBackToBackBranches::ID = 0;
+
+INITIALIZE_PASS(RISCVRemoveBackToBackBranches, DEBUG_TYPE,
+                "Fix hazards by removing back to back branches", false, false)
+
+/// Returns a pass that clears pipeline hazards.
+FunctionPass *llvm::createRISCVRemoveBackToBackBranches() {
+  return new RISCVRemoveBackToBackBranches();
+}
+
+static bool CheckCompressedISA(MachineBasicBlock *MBB,
+                               const RISCVInstrInfo *TII) {
+  unsigned SizeInBytes = 0;
+  for (auto &I : *MBB) {
+    // Skip some 0-sized meta instrucitons, such as debug ones.
+    if (!TII->getInstSizeInBytes(I))
+      continue;
+
+    SizeInBytes += TII->getInstSizeInBytes(I);
+
+    // This means that there is something other than the conditional branch
+    // here.
+    if (!I.isConditionalBranch())
+      continue;
+
+    // If it is a conditional branch, make sure it is the last one
+    // in this MBB and the cumulative size in bytes of other instructions in the
+    // block is <= 6 (since there potentially could be space for the two
+    // branches in the same 8-byte aligned code region, when compressed version
+    // of the instructions (16-bit size) is being used).
+    if (&I == &*MBB->getLastNonDebugInstr()) {
+      if (SizeInBytes <= 6)
+        return true;
+      return false;
+    }
+  }
+
+  return false;
+}
+
+static bool CheckNonCompressedISA(MachineBasicBlock *MBB,
+                                  const RISCVInstrInfo *TII) {
+  for (auto &I : *MBB) {
+    // Skip some 0-sized meta instrucitons, such as debug ones.
+    if (!TII->getInstSizeInBytes(I))
+      continue;
+
+    // This means that there is something other than the conditional branch
+    // here.
+    if (!I.isConditionalBranch())
+      return false;
+
+    // If it is a conditional branch, make sure it is the last one
+    // in this MBB.
+    if (&I == &*MBB->getLastNonDebugInstr())
+      return true;
+    return false;
+  }
+  return false;
+}
+
+bool RISCVRemoveBackToBackBranches::runOnMachineFunction(MachineFunction &MF) {
+  STI = &static_cast<const RISCVSubtarget &>(MF.getSubtarget());
+  TII = static_cast<const RISCVInstrInfo *>(STI->getInstrInfo());
+
+  if (!STI->shouldRemoveBackToBackBranches()) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "Ignoring RISCV Remove Back To Back Branches Pass\n");
+    return false;
+  }
+
+  bool Changed = false;
+  for (auto &MBB : MF) {
+    auto BBTerminator = MBB.getFirstTerminator();
+    // If it is not a conditional branch, we are not interested.
+    if (BBTerminator == MBB.end() ||
+        &*BBTerminator != &*MBB.getLastNonDebugInstr() ||
+        !BBTerminator->isConditionalBranch())
+      continue;
+
+    for (auto &Successor : MBB.successors()) {
+      // Set up aligment in order to avoid hazards. No 2 conditional branches
+      // should be in the same 8-byte aligned region of code. Similar to MIPS
+      // forbidden slots problem. We may want to insert a NOP only, but we
+      // need to think of Compressed ISA, so it is more safe to just set up
+      // aligment to the successor block if it meets requirements.
+      bool ShouldSetAligment = STI->getFeatureBits()[RISCV::FeatureStdExtC]
+                                   ? CheckCompressedISA(Successor, TII)
+                                   : CheckNonCompressedISA(Successor, TII);
+      if (ShouldSetAligment) {
+        Successor->setAlignment(Align(NumberOfBytesOfCodeRegion));
+        Changed = true;
+        ++NumInsertedAligments;
+      }
+    }
+  }
+
+  return Changed;
+}
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.cpp b/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -71,6 +71,11 @@ static cl::opt<bool> UseCCMovInsn("riscv-ccmov",
                                   cl::desc("RISCV: Use 'ccmov' instruction"),
                                   cl::init(true), cl::Hidden);
 
+static cl::opt<bool> RISCVRemoveBackToBackBranches(
+    "riscv-remove-back-to-back-branches",
+    cl::desc("RISCV: Insert nops to clear pipeline hazards."), cl::init(false),
+    cl::Hidden);
+
 void RISCVSubtarget::anchor() {}
 
 RISCVSubtarget &
@@ -233,3 +238,7 @@ bool RISCVSubtarget::useLoadStorePairs() const {
 bool RISCVSubtarget::useCCMovInsn() const {
   return UseCCMovInsn && HasMIPSCMov;
 }
+
+bool RISCVSubtarget::shouldRemoveBackToBackBranches() const {
+  return RISCVRemoveBackToBackBranches && hasFeature(RISCV::TuneMIPSP8700);
+}
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.h b/llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -169,9 +169,8 @@ class RISCVSubtarget : public RISCVGenSubtargetInfo {
   MVT getXLenVT() const {
     return is64Bit() ? MVT::i64 : MVT::i32;
   }
-  unsigned getXLen() const {
-    return is64Bit() ? 64 : 32;
-  }
+  unsigned getXLen() const { return is64Bit() ? 64 : 32; }
+  bool shouldRemoveBackToBackBranches() const;
   bool useLoadStorePairs() const;
   bool useCCMovInsn() const;
   unsigned getFLen() const {
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -579,13 +579,14 @@ void RISCVPassConfig::addPreEmitPass() {
   if (TM->getOptLevel() >= CodeGenOptLevel::Default &&
       EnableRISCVCopyPropagation)
     addPass(createMachineCopyPropagationPass(true));
-  addPass(&BranchRelaxationPassID);
   addPass(createRISCVMakeCompressibleOptPass());
 
   // LoadStoreOptimizer creates bundles for load-store bonding.
   addPass(createUnpackMachineBundles([](const MachineFunction &MF) {
     return MF.getSubtarget<RISCVSubtarget>().useLoadStorePairs();
   }));
+  addPass(&BranchRelaxationPassID);
+  addPass(createRISCVRemoveBackToBackBranches());
 }
 
 void RISCVPassConfig::addPreEmitPass2() {
diff --git a/llvm/test/CodeGen/MIR/RISCV/riscv-remove-back-to-back-branches.mir b/llvm/test/CodeGen/MIR/RISCV/riscv-remove-back-to-back-branches.mir
@@ -0,0 +1,108 @@
+# RUN: llc -mtriple=riscv64 -mattr=-c -riscv-remove-back-to-back-branches=1 -o - %s | FileCheck %s
+
+# CHECK: %bb.0:
+# CHECK: blez
+# CHECK: .p2align 3
+# CHECK: %bb.1:
+# CHECK: blez
+
+--- |
+  ; ModuleID = 'hazaard.c'
+  source_filename = "hazaard.c"
+  target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
+  target triple = "riscv64-unknown-linux-gnu"
+  
+  ; Function Attrs: nounwind optsize
+  define dso_local void @test(i32 noundef signext %a, i32 noundef signext %b) local_unnamed_addr #0 {
+  entry:
+    %cmp = icmp sgt i32 %a, 0
+    br i1 %cmp, label %if.then, label %if.end3
+  
+  if.then:                                          ; preds = %entry
+    %cmp1 = icmp slt i32 %b, 1
+    br i1 %cmp1, label %if.then2, label %if.end3
+  
+  if.then2:                                         ; preds = %if.then
+    tail call void asm sideeffect "nop", ""() #1, !srcloc !4
+    ret void
+  
+  if.end3:                                          ; preds = %if.then, %entry
+    ret void
+  }
+  
+  attributes #0 = { nounwind optsize "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="mips-p8700" "target-features"="+64bit,+a,+c,+d,+f,+m" }
+  attributes #1 = { nounwind }
+  
+  !llvm.module.flags = !{!0, !1, !2}
+  !llvm.ident = !{!3}
+  
+  !0 = !{i32 1, !"wchar_size", i32 4}
+  !1 = !{i32 1, !"target-abi", !"lp64d"}
+  !2 = !{i32 1, !"SmallDataLimit", i32 8}
+  !3 = !{!"clang version 14.0.0 (git@github.com:MIPS/llvm.git ae54cf4034587fab977092097c9772c7a275ddc8)"}
+  !4 = !{i64 88}
+
+...
+---
+name:            test
+alignment:       2
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+failsVerification: false
+tracksDebugUserValues: true
+registers:       []
+liveins:
+  - { reg: '$x10', virtual-reg: '' }
+  - { reg: '$x11', virtual-reg: '' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    1
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  hasTailCall:     false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+callSites:       []
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x50000000), %bb.2(0x30000000)
+    liveins: $x10, $x11
+  
+    BGE $x0, killed renamable $x10, %bb.2
+  
+  bb.1.if.then:
+    successors: %bb.3(0x30000000), %bb.2(0x50000000)
+    liveins: $x11
+  
+    BGE $x0, killed renamable $x11, %bb.3
+  
+  bb.2.if.end3:
+    PseudoRET
+  
+  bb.3.if.then2:
+    INLINEASM &nop, 1 /* sideeffect attdialect */, !4
+    PseudoRET
+
+...
diff --git a/llvm/test/CodeGen/RISCV/O0-pipeline.ll b/llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -61,9 +61,10 @@
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
-; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       RISC-V Make Compressible
 ; CHECK-NEXT:       Unpack machine instruction bundles
+; CHECK-NEXT:       Branch relaxation pass
+; CHECK-NEXT:       RISCV Remove Back To Back Branches Pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       Remove Loads Into Fake Uses
 ; CHECK-NEXT:       StackMap Liveness Analysis
diff --git a/llvm/test/CodeGen/RISCV/O3-pipeline.ll b/llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -188,9 +188,10 @@
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       Machine Copy Propagation Pass
-; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       RISC-V Make Compressible
 ; CHECK-NEXT:       Unpack machine instruction bundles
+; CHECK-NEXT:       Branch relaxation pass
+; CHECK-NEXT:       RISCV Remove Back To Back Branches Pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       Remove Loads Into Fake Uses
 ; CHECK-NEXT:       StackMap Liveness Analysis

Original file line number	Diff line number	Diff line change
`@@ -2178,6 +2178,10 @@ void Clang::AddRISCVTargetArgs(const ArgList &Args,`
`2178`	`2178`	`CmdArgs.push_back("-riscv-ccmov=0");`
`2179`	`2179`	`}`
`2180`	`2180`	`}`
	`2181`	`+ if (Args.getLastArg(options::OPT_mremove_back_to_back_branches)) {`
	`2182`	`+ CmdArgs.push_back("-mllvm");`
	`2183`	`+ CmdArgs.push_back("-riscv-remove-back-to-back-branches=1");`
	`2184`	`+ }`
`2181`	`2185`	`// Handle -mrvv-vector-bits=<bits>`
`2182`	`2186`	`if (Arg *A = Args.getLastArg(options::OPT_mrvv_vector_bits_EQ)) {`
`2183`	`2187`	`StringRef Val = A->getValue();`