[PATCH] D119305: [AArch64][LoadStoreOptimizer] Make sure physical registers used by renamable undef are not picked as register to rename.

Huihui Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 8 17:25:23 PST 2022


huihuiz created this revision.
huihuiz added reviewers: ostannard, dmgreen, efriedma, samparker, fhahn.
huihuiz added a project: LLVM.
Herald added subscribers: hiraditya, kristof.beyls.
huihuiz requested review of this revision.

When handling store pairs, we may need to find a renamable register to enable
merging store pairs. Make sure physical registers used by renamable undef for
instructions in between store pairs are not selected as register to rename.
Otherwise, it will trigger assertion on register overlap.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119305

Files:
  llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
  llvm/test/CodeGen/AArch64/stp-opt-with-renaming-crash.mir


Index: llvm/test/CodeGen/AArch64/stp-opt-with-renaming-crash.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stp-opt-with-renaming-crash.mir
@@ -0,0 +1,63 @@
+# RUN: llc -run-pass=aarch64-ldst-opt -mtriple=aarch64 -verify-machineinstrs -o - %s | FileCheck %s
+
+# This test checks that aarch64 load store optimizer is not crashing with:
+# "Rename register used between paired instruction, trashing the content".
+# The crash was caused by picking renamable register q16, which is not used
+# as defs, not liveins, not reserved or callee preserved and can be used for
+# all class. However overlap with renamable undef d16 used by ZIP2 instruction.
+
+# This test also checks that pairwise store STP is generated.
+
+# CHECK-LABLE: test
+# CHECK: bb.0:
+# CHECK-NEXT: liveins: $x0, $x17, $x18
+# CHECK: renamable $q13_q14_q15 = LD3Threev16b undef renamable $x17 :: (load (s384) from `<16 x i8>* undef`, align 64)
+# CHECK-NEXT: renamable $q23_q24_q25 = LD3Threev16b undef renamable $x18 :: (load (s384) from `<16 x i8>* undef`, align 64)
+# CHECK-NEXT: $q17 = EXTv16i8 renamable $q23, renamable $q23, 8
+# CHECK-NEXT: renamable $q20 = EXTv16i8 renamable $q14, renamable $q14, 8
+# CHECK-NEXT: STRQui killed renamable $q20, $sp, 4 :: (store (s128) into %stack.3)
+# CHECK-NEXT: renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d16
+# CHECK-NEXT: STRDui killed renamable $d6, $sp, 11 :: (store (s64) into %stack.2)
+# CHECK-NEXT: renamable $q6 = EXTv16i8 renamable $q13, renamable $q13, 8
+# CHECK-NEXT: STPQi killed renamable $q6, killed $q17, $sp, 6 :: (store (s128) into %stack.0), (store (s128) into %stack.1)
+# CHECK-NEXT: RET undef $lr
+
+---
+name:            test
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0', virtual-reg: '' }
+frameInfo:
+  maxAlignment:    16
+  maxCallFrameSize: 0
+fixedStack:      []
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -80, size: 16, alignment: 16,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, name: '', type: spill-slot, offset: -96, size: 16, alignment: 16,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 2, name: '', type: spill-slot, offset: -104, size: 8, alignment: 8,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 3, name: '', type: spill-slot, offset: -128, size: 16, alignment: 16,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0, $x17, $x18
+    renamable $q13_q14_q15 = LD3Threev16b undef renamable $x17 :: (load (s384) from `<16 x i8>* undef`, align 64)
+    renamable $q23_q24_q25 = LD3Threev16b undef renamable $x18 :: (load (s384) from `<16 x i8>* undef`, align 64)
+    renamable $q20 = EXTv16i8 renamable $q23, renamable $q23, 8
+    STRQui killed renamable $q20, $sp, 7 :: (store (s128) into %stack.0)
+    renamable $q20 = EXTv16i8 renamable $q14, renamable $q14, 8
+    STRQui killed renamable $q20, $sp, 4 :: (store (s128) into %stack.3)
+    renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d16
+    STRDui killed renamable $d6, $sp, 11 :: (store (s64) into %stack.2)
+    renamable $q6 = EXTv16i8 renamable $q13, renamable $q13, 8
+    STRQui killed renamable $q6, $sp, 6 :: (store (s128) into %stack.1)
+    RET undef $lr
+...
Index: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
+++ llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
@@ -1499,11 +1499,26 @@
     });
   };
 
+  // Checks if any instr in between FirstMI and MI has renamable undef that
+  // overlap with PR.
+  auto UsedByUndefInBetween = [TRI](MachineInstr &FirstMI, MachineInstr &MI,
+                                    const MCPhysReg &PR) {
+    for (auto &MI :
+         make_range(std::next(FirstMI.getIterator()), MI.getIterator())) {
+      for (auto &MOP : MI.operands()) {
+        if (MOP.isReg() && MOP.isUndef() && MOP.isRenamable() &&
+            TRI->regsOverlap(MOP.getReg(), PR))
+          return true;
+      }
+    }
+    return false;
+  };
+
   auto *RegClass = TRI->getMinimalPhysRegClass(getLdStRegOp(FirstMI).getReg());
   for (const MCPhysReg &PR : *RegClass) {
     if (DefinedInBB.available(PR) && UsedInBetween.available(PR) &&
         !RegInfo.isReserved(PR) && !AnySubOrSuperRegCalleePreserved(PR) &&
-        CanBeUsedForAllClasses(PR)) {
+        CanBeUsedForAllClasses(PR) && !UsedByUndefInBetween(FirstMI, MI, PR)) {
       DefinedInBB.addReg(PR);
       LLVM_DEBUG(dbgs() << "Found rename register " << printReg(PR, TRI)
                         << "\n");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119305.407016.patch
Type: text/x-patch
Size: 5182 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220209/18cf6e5e/attachment.bin>


More information about the llvm-commits mailing list