Skip to content

Conversation

wangleiat
Copy link
Contributor

This patch fixes the crash issue in the test:
CodeGen/LoongArch/can-not-realign-stack.ll

Register allocator may spill virtual registers to the stack, which
introduces stack alignment requirements (when the size of spilled
registers exceeds the default alignment size of the stack). If a
function does not have stack alignment requirements before register
allocation, registers used for stack alignment will not be preserved.

Therefore, we should implement canRealignStack() to inform the
register allocator whether it is allowed to perform stack realignment
operations.

@llvmbot
Copy link
Member

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-backend-loongarch

Author: wanglei (wangleiat)

Changes

This patch fixes the crash issue in the test:
CodeGen/LoongArch/can-not-realign-stack.ll

Register allocator may spill virtual registers to the stack, which
introduces stack alignment requirements (when the size of spilled
registers exceeds the default alignment size of the stack). If a
function does not have stack alignment requirements before register
allocation, registers used for stack alignment will not be preserved.

Therefore, we should implement canRealignStack() to inform the
register allocator whether it is allowed to perform stack realignment
operations.


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

3 Files Affected:

  • (modified) llvm/lib/Target/LoongArch/LoongArchRegisterInfo.cpp (+23)
  • (modified) llvm/lib/Target/LoongArch/LoongArchRegisterInfo.h (+1)
  • (added) llvm/test/CodeGen/LoongArch/can-not-realign-stack.ll (+85)
diff --git a/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.cpp b/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.cpp
index 257b947a3ce436..092b5f1fb44261 100644
--- a/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.cpp
@@ -15,6 +15,7 @@
 #include "LoongArch.h"
 #include "LoongArchInstrInfo.h"
 #include "LoongArchSubtarget.h"
+#include "MCTargetDesc/LoongArchBaseInfo.h"
 #include "MCTargetDesc/LoongArchMCTargetDesc.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -194,3 +195,25 @@ bool LoongArchRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
   MI.getOperand(FIOperandNum + 1).ChangeToImmediate(Offset.getFixed());
   return false;
 }
+
+bool LoongArchRegisterInfo::canRealignStack(const MachineFunction &MF) const {
+  if (!TargetRegisterInfo::canRealignStack(MF))
+    return false;
+
+  const MachineRegisterInfo *MRI = &MF.getRegInfo();
+  const LoongArchFrameLowering *TFI = getFrameLowering(MF);
+
+  // Stack realignment requires a frame pointer.  If we already started
+  // register allocation with frame pointer elimination, it is too late now.
+  if (!MRI->canReserveReg(LoongArch::R22))
+    return false;
+
+  // We may also need a base pointer if there are dynamic allocas or stack
+  // pointer adjustments around calls.
+  if (TFI->hasReservedCallFrame(MF))
+    return true;
+
+  // A base pointer is required and allowed.  Check that it isn't too late to
+  // reserve it.
+  return MRI->canReserveReg(LoongArchABI::getBPReg());
+}
diff --git a/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.h b/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.h
index 7e8f26b1409765..d1e40254c2972e 100644
--- a/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.h
+++ b/llvm/lib/Target/LoongArch/LoongArchRegisterInfo.h
@@ -51,6 +51,7 @@ struct LoongArchRegisterInfo : public LoongArchGenRegisterInfo {
   bool requiresFrameIndexScavenging(const MachineFunction &MF) const override {
     return true;
   }
+  bool canRealignStack(const MachineFunction &MF) const override;
 };
 } // end namespace llvm
 
diff --git a/llvm/test/CodeGen/LoongArch/can-not-realign-stack.ll b/llvm/test/CodeGen/LoongArch/can-not-realign-stack.ll
new file mode 100644
index 00000000000000..af24ae64b7c741
--- /dev/null
+++ b/llvm/test/CodeGen/LoongArch/can-not-realign-stack.ll
@@ -0,0 +1,85 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc --mtriple=loongarch64 --frame-pointer=none --mattr=+lasx < %s | FileCheck %s
+
+;; This test is checking that when a function allows stack realignment and
+;; realignment needs were not detected before register allocation (at this
+;; point, fp is not preserved), but realignment is required during register
+;; allocation, the stack should not undergo realignment.
+
+;; Ensure that the `bstrins.d $sp, $zero, n, 0` instruction is not generated.
+;; n = log2(realign_size) - 1
+
+%struct.S = type { [64 x i16] }
+
+define dso_local noundef signext i32 @main() nounwind {
+; CHECK-LABEL: main:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    addi.d $sp, $sp, -272
+; CHECK-NEXT:    st.d $ra, $sp, 264 # 8-byte Folded Spill
+; CHECK-NEXT:    st.d $fp, $sp, 256 # 8-byte Folded Spill
+; CHECK-NEXT:    pcalau12i $a0, %pc_hi20(.LCPI0_0)
+; CHECK-NEXT:    addi.d $a0, $a0, %pc_lo12(.LCPI0_0)
+; CHECK-NEXT:    xvld $xr0, $a0, 0
+; CHECK-NEXT:    xvst $xr0, $sp, 96 # 32-byte Folded Spill
+; CHECK-NEXT:    pcalau12i $a0, %pc_hi20(.LCPI0_1)
+; CHECK-NEXT:    addi.d $a0, $a0, %pc_lo12(.LCPI0_1)
+; CHECK-NEXT:    xvld $xr1, $a0, 0
+; CHECK-NEXT:    xvst $xr1, $sp, 64 # 32-byte Folded Spill
+; CHECK-NEXT:    xvst $xr1, $sp, 224
+; CHECK-NEXT:    xvst $xr0, $sp, 192
+; CHECK-NEXT:    pcalau12i $a0, %pc_hi20(.LCPI0_2)
+; CHECK-NEXT:    addi.d $a0, $a0, %pc_lo12(.LCPI0_2)
+; CHECK-NEXT:    xvld $xr0, $a0, 0
+; CHECK-NEXT:    xvst $xr0, $sp, 32 # 32-byte Folded Spill
+; CHECK-NEXT:    xvst $xr0, $sp, 160
+; CHECK-NEXT:    pcalau12i $a0, %pc_hi20(.LCPI0_3)
+; CHECK-NEXT:    addi.d $a0, $a0, %pc_lo12(.LCPI0_3)
+; CHECK-NEXT:    xvld $xr0, $a0, 0
+; CHECK-NEXT:    xvst $xr0, $sp, 0 # 32-byte Folded Spill
+; CHECK-NEXT:    xvst $xr0, $sp, 128
+; CHECK-NEXT:    addi.d $fp, $sp, 128
+; CHECK-NEXT:    move $a0, $fp
+; CHECK-NEXT:    bl %plt(foo)
+; CHECK-NEXT:    xvld $xr0, $sp, 64 # 32-byte Folded Reload
+; CHECK-NEXT:    xvst $xr0, $sp, 224
+; CHECK-NEXT:    xvld $xr0, $sp, 96 # 32-byte Folded Reload
+; CHECK-NEXT:    xvst $xr0, $sp, 192
+; CHECK-NEXT:    xvld $xr0, $sp, 32 # 32-byte Folded Reload
+; CHECK-NEXT:    xvst $xr0, $sp, 160
+; CHECK-NEXT:    xvld $xr0, $sp, 0 # 32-byte Folded Reload
+; CHECK-NEXT:    xvst $xr0, $sp, 128
+; CHECK-NEXT:    move $a0, $fp
+; CHECK-NEXT:    bl %plt(bar)
+; CHECK-NEXT:    move $a0, $zero
+; CHECK-NEXT:    ld.d $fp, $sp, 256 # 8-byte Folded Reload
+; CHECK-NEXT:    ld.d $ra, $sp, 264 # 8-byte Folded Reload
+; CHECK-NEXT:    addi.d $sp, $sp, 272
+; CHECK-NEXT:    ret
+entry:
+  %s = alloca %struct.S, align 2
+  call void @llvm.lifetime.start.p0(i64 128, ptr nonnull %s)
+  store <16 x i16> <i16 16384, i16 16129, i16 15874, i16 15619, i16 15364, i16 15109, i16 14854, i16 14599, i16 14344, i16 14089, i16 13834, i16 13579, i16 13324, i16 13069, i16 12814, i16 12559>, ptr %s, align 2
+  %0 = getelementptr inbounds [64 x i16], ptr %s, i64 0, i64 16
+  store <16 x i16> <i16 12304, i16 12049, i16 11794, i16 11539, i16 11284, i16 11029, i16 10774, i16 10519, i16 10264, i16 10009, i16 9754, i16 9499, i16 9244, i16 8989, i16 8734, i16 8479>, ptr %0, align 2
+  %1 = getelementptr inbounds [64 x i16], ptr %s, i64 0, i64 32
+  store <16 x i16> <i16 8224, i16 7969, i16 7714, i16 7459, i16 7204, i16 6949, i16 6694, i16 6439, i16 6184, i16 5929, i16 5674, i16 5419, i16 5164, i16 4909, i16 4654, i16 4399>, ptr %1, align 2
+  %2 = getelementptr inbounds [64 x i16], ptr %s, i64 0, i64 48
+  store <16 x i16> <i16 4144, i16 3889, i16 3634, i16 3379, i16 3124, i16 2869, i16 2614, i16 2359, i16 2104, i16 1849, i16 1594, i16 1339, i16 1084, i16 829, i16 574, i16 319>, ptr %2, align 2
+  call void @foo(ptr noundef nonnull %s)
+  store <16 x i16> <i16 16384, i16 16129, i16 15874, i16 15619, i16 15364, i16 15109, i16 14854, i16 14599, i16 14344, i16 14089, i16 13834, i16 13579, i16 13324, i16 13069, i16 12814, i16 12559>, ptr %s, align 2
+  %3 = getelementptr inbounds [64 x i16], ptr %s, i64 0, i64 16
+  store <16 x i16> <i16 12304, i16 12049, i16 11794, i16 11539, i16 11284, i16 11029, i16 10774, i16 10519, i16 10264, i16 10009, i16 9754, i16 9499, i16 9244, i16 8989, i16 8734, i16 8479>, ptr %3, align 2
+  %4 = getelementptr inbounds [64 x i16], ptr %s, i64 0, i64 32
+  store <16 x i16> <i16 8224, i16 7969, i16 7714, i16 7459, i16 7204, i16 6949, i16 6694, i16 6439, i16 6184, i16 5929, i16 5674, i16 5419, i16 5164, i16 4909, i16 4654, i16 4399>, ptr %4, align 2
+  %5 = getelementptr inbounds [64 x i16], ptr %s, i64 0, i64 48
+  store <16 x i16> <i16 4144, i16 3889, i16 3634, i16 3379, i16 3124, i16 2869, i16 2614, i16 2359, i16 2104, i16 1849, i16 1594, i16 1339, i16 1084, i16 829, i16 574, i16 319>, ptr %5, align 2
+  call void @bar(ptr noundef nonnull %s)
+  call void @llvm.lifetime.end.p0(i64 128, ptr nonnull %s)
+  ret i32 0
+}
+
+declare void @foo(ptr nocapture noundef)
+declare void @bar(ptr nocapture noundef)
+
+declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture)
+declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture)

@wangleiat
Copy link
Contributor Author

@xry111 @xen0n

Copy link
Contributor

@SixWeining SixWeining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@xen0n xen0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's based on some other target's similar treatment it could be useful to refer to that arch, otherwise it LGTM as-is.

wangleiat added a commit that referenced this pull request Jan 9, 2024
This test will crash with expensive check.

Crash message:
```
*** Bad machine code: Using an undefined physical register ***
- function:    main
- basic block: %bb.0 entry (0x20fee70)
- instruction: $r3 = frame-destroy ADDI_D $r22, -288
- operand 1:   $r22
```
This patch fixes the crash issue in the test:
CodeGen/LoongArch/can-not-realign-stack.ll

Register allocator may spill virtual registers to the stack, which
introduces stack alignment requirements (when the size of spilled
    registers exceeds the default alignment size of the stack). If a
function does not have stack alignment requirements before register
allocation, registers used for stack alignment will not be preserved.

Therefore, we should implement `canRealignStack()` to inform the
register allocator whether it is allowed to perform stack realignment
operations.
@wangleiat wangleiat merged commit 98c6aa7 into llvm:main Jan 9, 2024
@wangleiat wangleiat deleted the realign branch January 22, 2024 06:09
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This test will crash with expensive check.

Crash message:
```
*** Bad machine code: Using an undefined physical register ***
- function:    main
- basic block: %bb.0 entry (0x20fee70)
- instruction: $r3 = frame-destroy ADDI_D $r22, -288
- operand 1:   $r22
```
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…76913)

This patch fixes the crash issue in the test:
CodeGen/LoongArch/can-not-realign-stack.ll

Register allocator may spill virtual registers to the stack, which    
introduces stack alignment requirements (when the size of spilled     
    registers exceeds the default alignment size of the stack). If a  
function does not have stack alignment requirements before register   
allocation, registers used for stack alignment will not be preserved. 

Therefore, we should implement `canRealignStack()` to inform the      
register allocator whether it is allowed to perform stack realignment 
operations.
leecheechen pushed a commit to leecheechen/llvm-project that referenced this pull request Jun 9, 2025
This test will crash with expensive check.

Crash message:
```
*** Bad machine code: Using an undefined physical register ***
- function:    main
- basic block: %bb.0 entry (0x20fee70)
- instruction: $r3 = frame-destroy ADDI_D $r22, -288
- operand 1:   $r22
```

(cherry picked from commit f499472)

Change-Id: I91aa251ada69ef63ce6f80ff203540b0cb55871a
leecheechen pushed a commit to leecheechen/llvm-project that referenced this pull request Jun 9, 2025
…76913)

This patch fixes the crash issue in the test:
CodeGen/LoongArch/can-not-realign-stack.ll

Register allocator may spill virtual registers to the stack, which
introduces stack alignment requirements (when the size of spilled
    registers exceeds the default alignment size of the stack). If a
function does not have stack alignment requirements before register
allocation, registers used for stack alignment will not be preserved.

Therefore, we should implement `canRealignStack()` to inform the
register allocator whether it is allowed to perform stack realignment
operations.

(cherry picked from commit 98c6aa7)

Change-Id: I43d38bf321b595a1f6a4899111db4f71660a2d9a
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.

4 participants