[Hexagon] Enable Machine Combiner pass.

[sgundapa]

No description provided.

[llvmbot]

@llvm/pr-subscribers-backend-hexagon

Author: Sumanth Gundapaneni (sgundapa)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/169434.diff

4 Files Affected:

• (modified) llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp (+14)
• (modified) llvm/lib/Target/Hexagon/HexagonInstrInfo.h (+3)
• (modified) llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp (+11)
• (added) llvm/test/CodeGen/Hexagon/machine-combiner-reassoc.mir (+65)

diff --git a/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp b/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
index 7682af4543b7c..7a7e5e29e1ad0 100644
--- a/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonInstrInfo.cpp
@@ -1808,6 +1808,20 @@ bool HexagonInstrInfo::isPredicable(const MachineInstr &MI) const {
   return true;
 }
 
+bool HexagonInstrInfo::isAssociativeAndCommutative(const MachineInstr &Inst,
+                                                   bool Invert) const {
+  if (Invert)
+    return false;
+
+  switch (Inst.getOpcode()) {
+  // TODO: Add more instructions to be handled by MachineCombiner.
+  case Hexagon::F2_sfadd:
+    return Inst.getFlag(MachineInstr::MIFlag::FmReassoc);
+  default:
+    return false;
+  }
+}
+
 bool HexagonInstrInfo::isSchedulingBoundary(const MachineInstr &MI,
                                             const MachineBasicBlock *MBB,
                                             const MachineFunction &MF) const {
diff --git a/llvm/lib/Target/Hexagon/HexagonInstrInfo.h b/llvm/lib/Target/Hexagon/HexagonInstrInfo.h
index 796b978a2c3f0..29d45fd85ac43 100644
--- a/llvm/lib/Target/Hexagon/HexagonInstrInfo.h
+++ b/llvm/lib/Target/Hexagon/HexagonInstrInfo.h
@@ -505,6 +505,9 @@ class HexagonInstrInfo : public HexagonGenInstrInfo {
                              bool ToBigInstrs = true) const;
   void translateInstrsForDup(MachineBasicBlock::instr_iterator MII,
                              bool ToBigInstrs) const;
+  bool useMachineCombiner() const override { return true; }
+  bool isAssociativeAndCommutative(const MachineInstr &Inst,
+                                   bool Invert) const override;
 
   // Addressing mode relations.
   short changeAddrMode_abs_io(short Opc) const;
diff --git a/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp b/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
index d9824a3154093..70d87a0a62a21 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
@@ -47,6 +47,9 @@ static cl::opt<bool>
     DisableHardwareLoops("disable-hexagon-hwloops", cl::Hidden,
                          cl::desc("Disable Hardware Loops for Hexagon target"));
 
+static cl::opt<bool> EnableMCR("hexagon-mcr", cl::Hidden, cl::init(true),
+                               cl::desc("Enable the machine combiner pass"));
+
 static cl::opt<bool>
     DisableAModeOpt("disable-hexagon-amodeopt", cl::Hidden,
                     cl::desc("Disable Hexagon Addressing Mode Optimization"));
@@ -309,6 +312,7 @@ class HexagonPassConfig : public TargetPassConfig {
 
   void addIRPasses() override;
   bool addInstSelector() override;
+  bool addILPOpts() override;
   void addPreRegAlloc() override;
   void addPostRegAlloc() override;
   void addPreSched2() override;
@@ -393,6 +397,13 @@ bool HexagonPassConfig::addInstSelector() {
   return false;
 }
 
+bool HexagonPassConfig::addILPOpts() {
+  if (EnableMCR)
+    addPass(&MachineCombinerID);
+
+  return true;
+}
+
 void HexagonPassConfig::addPreRegAlloc() {
   if (getOptLevel() != CodeGenOptLevel::None) {
     if (EnableCExtOpt)
diff --git a/llvm/test/CodeGen/Hexagon/machine-combiner-reassoc.mir b/llvm/test/CodeGen/Hexagon/machine-combiner-reassoc.mir
new file mode 100644
index 0000000000000..275f1e79c8cc4
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/machine-combiner-reassoc.mir
@@ -0,0 +1,65 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
+# RUN: llc -mtriple=hexagon -hexagon-mcr=true -run-pass=machine-combiner\
+# RUN:  %s -o - | FileCheck %s --check-prefix=ENABLED
+# RUN: llc -mtriple=hexagon -hexagon-mcr=false -run-pass=machine-combiner\
+# RUN:  %s -o - | FileCheck %s --check-prefix=DISABLED
+
+# Test that demonstrates the machine-combiner pass performing reassociation
+# to improve instruction-level parallelism (ILP) by breaking dependency chains.
+
+--- |
+  define float @test_reassoc_chain(float* nocapture readonly %a) {
+    ret float 0.0
+  }
+...
+
+---
+# Test a longer chain: ((A + B) + C) + D
+# Should reassociate to optimize the critical path
+# Verify we still have 3 F2_sfadd instructions but in optimized order
+# Original chain should be preserved
+
+name: test_reassoc_chain
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $r0
+
+    ; ENABLED-LABEL: name: test_reassoc_chain
+    ; ENABLED: liveins: $r0
+    ; ENABLED-NEXT: {{  $}}
+    ; ENABLED-NEXT: [[COPY:%[0-9]+]]:intregs = COPY $r0
+    ; ENABLED-NEXT: [[L2_loadri_io:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 0
+    ; ENABLED-NEXT: [[L2_loadri_io1:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 4
+    ; ENABLED-NEXT: [[F2_sfadd:%[0-9]+]]:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed [[L2_loadri_io1]], killed [[L2_loadri_io]], implicit $usr
+    ; ENABLED-NEXT: [[L2_loadri_io2:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 8
+    ; ENABLED-NEXT: [[L2_loadri_io3:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 12
+    ; ENABLED-NEXT: [[F2_sfadd1:%[0-9]+]]:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed [[L2_loadri_io2]], killed [[L2_loadri_io3]], implicit $usr
+    ; ENABLED-NEXT: [[F2_sfadd2:%[0-9]+]]:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed [[F2_sfadd]], killed [[F2_sfadd1]], implicit $usr
+    ; ENABLED-NEXT: $r0 = COPY [[F2_sfadd2]]
+    ; ENABLED-NEXT: PS_jmpret $r31, implicit-def dead $pc, implicit $r0
+    ;
+    ; DISABLED-LABEL: name: test_reassoc_chain
+    ; DISABLED: liveins: $r0
+    ; DISABLED-NEXT: {{  $}}
+    ; DISABLED-NEXT: [[COPY:%[0-9]+]]:intregs = COPY $r0
+    ; DISABLED-NEXT: [[L2_loadri_io:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 0
+    ; DISABLED-NEXT: [[L2_loadri_io1:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 4
+    ; DISABLED-NEXT: [[F2_sfadd:%[0-9]+]]:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed [[L2_loadri_io1]], killed [[L2_loadri_io]], implicit $usr
+    ; DISABLED-NEXT: [[L2_loadri_io2:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 8
+    ; DISABLED-NEXT: [[L2_loadri_io3:%[0-9]+]]:intregs = L2_loadri_io [[COPY]], 12
+    ; DISABLED-NEXT: [[F2_sfadd1:%[0-9]+]]:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed [[L2_loadri_io2]], killed [[L2_loadri_io3]], implicit $usr
+    ; DISABLED-NEXT: [[F2_sfadd2:%[0-9]+]]:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed [[F2_sfadd]], killed [[F2_sfadd1]], implicit $usr
+    ; DISABLED-NEXT: $r0 = COPY [[F2_sfadd2]]
+    ; DISABLED-NEXT: PS_jmpret $r31, implicit-def dead $pc, implicit $r0
+    %0:intregs = COPY $r0
+    %1:intregs = L2_loadri_io %0:intregs, 0
+    %2:intregs = L2_loadri_io %0:intregs, 4
+    %3:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed %2:intregs, killed %1:intregs, implicit $usr
+    %4:intregs = L2_loadri_io %0:intregs, 8
+    %5:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed %3:intregs, killed %4:intregs, implicit $usr
+    %6:intregs = L2_loadri_io %0:intregs, 12
+    %7:intregs = nnan ninf nsz arcp contract afn reassoc F2_sfadd killed %5:intregs, killed %6:intregs, implicit $usr
+    $r0 = COPY %7:intregs
+    PS_jmpret $r31, implicit-def dead $pc, implicit $r0
+...

[sgundapa]

This patch will reduce some downstream delta and will be a crucial piece in some of the optimizations

[androm3da]

This patch will reduce some downstream delta and will be a crucial piece in some of the optimizations

Is this pass on by default in ~typical optimization levels like -O1/-O2? If so, can you run the llvm-test-suite to check for regressions?

[sgundapa]

This patch will reduce some downstream delta and will be a crucial piece in some of the optimizations

Is this pass on by default in ~typical optimization levels like -O1/-O2? If so, can you run the llvm-test-suite to check for regressions?

Yes. It is on by default.