Skip to content

Conversation

@sdesmalen-arm
Copy link
Collaborator

The LoadStoreOptimizer is very conservative with handling instructions that have implicit-def operands and only support them for 2 instructions. However, they can be considered when a MachineOperand is marked explicitly as 'renamable'.

The LoadStoreOptimizer is very conservative with handling instructions
that have implicit-def operands, and only support them for 2 instructions.
However, they can be considered also when marked explicitly as 'renamable'.
@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2026

@llvm/pr-subscribers-backend-aarch64

Author: Sander de Smalen (sdesmalen-arm)

Changes

The LoadStoreOptimizer is very conservative with handling instructions that have implicit-def operands and only support them for 2 instructions. However, they can be considered when a MachineOperand is marked explicitly as 'renamable'.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp (+5-5)
  • (modified) llvm/test/CodeGen/AArch64/ldst-implicitop.mir (+29)
diff --git a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
index 45599de6a4828..3d9444c0c5426 100644
--- a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
+++ b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
@@ -833,10 +833,10 @@ static bool isMergeableIndexLdSt(MachineInstr &MI, int &Scale) {
   }
 }
 
-static bool isRewritableImplicitDef(unsigned Opc) {
-  switch (Opc) {
+static bool isRewritableImplicitDef(const MachineOperand &MO) {
+  switch (MO.getParent()->getOpcode()) {
   default:
-    return false;
+    return MO.isRenamable();
   case AArch64::ORRWrs:
   case AArch64::ADDWri:
     return true;
@@ -1047,7 +1047,7 @@ AArch64LoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
                         MI.getRegClassConstraint(OpIdx, TII, TRI))
                   MatchingReg = GetMatchingSubReg(RC);
                 else {
-                  if (!isRewritableImplicitDef(MI.getOpcode()))
+                  if (!isRewritableImplicitDef(MOP))
                     continue;
                   MatchingReg = GetMatchingSubReg(
                       TRI->getMinimalPhysRegClass(MOP.getReg()));
@@ -1739,7 +1739,7 @@ static bool canRenameMOP(const MachineOperand &MOP,
     // them must be known. For example, in ORRWrs the implicit-def
     // corresponds to the result register.
     if (MOP.isImplicit() && MOP.isDef()) {
-      if (!isRewritableImplicitDef(MOP.getParent()->getOpcode()))
+      if (!isRewritableImplicitDef(MOP))
         return false;
       return TRI->isSuperOrSubRegisterEq(
           MOP.getParent()->getOperand(0).getReg(), MOP.getReg());
diff --git a/llvm/test/CodeGen/AArch64/ldst-implicitop.mir b/llvm/test/CodeGen/AArch64/ldst-implicitop.mir
index 34e8cf282669c..482ae5894a5d8 100644
--- a/llvm/test/CodeGen/AArch64/ldst-implicitop.mir
+++ b/llvm/test/CodeGen/AArch64/ldst-implicitop.mir
@@ -78,3 +78,32 @@ body:             |
     $q1 = ORRv16i8 $q5, killed $q5
     RET_ReallyLR
 ...
+# Test that when the implicit-def is renamable, the loads/stores can still be
+# bundled together.
+---
+name:            impdef_renamable
+tracksRegLiveness: true
+stack:
+  - { id: 0, name: '', type: default, offset: -8, size: 8, alignment: 8,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -8, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+  - { id: 1, name: '', type: default, offset: -16, size: 8, alignment: 8,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -16, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: impdef_renamable
+    ; CHECK: early-clobber $sp, renamable $w8, $w9 = frame-setup LDPWpre $sp, -4 :: (load (s32) from %stack.1 + 4), (load (s32) from %stack.1, align 8)
+    ; CHECK-NEXT: STPWi killed renamable $w8, killed $w9, $sp, 2 :: (store (s32) into %stack.0 + 4), (store (s32) into %stack.0, align 8)
+    ; CHECK-NEXT: $sp = frame-destroy ADDXri $sp, 16, 0
+    ; CHECK-NEXT: RET undef $lr
+    $sp = frame-setup SUBXri $sp, 16, 0
+    renamable $w8 = LDRWui $sp, 1, implicit-def renamable $x8 :: (load (s32) from %stack.1 + 4)
+    STRWui killed renamable $w8, $sp, 3 :: (store (s32) into %stack.0 + 4)
+    renamable $w8 = LDRWui $sp, 0, implicit-def renamable $x8 :: (load (s32) from %stack.1, align 8)
+    STRWui killed renamable $w8, $sp, 2 :: (store (s32) into %stack.0, align 8)
+    $sp = frame-destroy ADDXri $sp, 16, 0
+    RET undef $lr
+...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants