[llvm] [ARM][ConstantIslands] Correct MinNoSplitDisp calculation (PR #114590)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 11 12:22:00 PST 2024
https://github.com/pzhengqc updated https://github.com/llvm/llvm-project/pull/114590
>From e19cbcd6da93b5696c99278ccf0027cb441b9e16 Mon Sep 17 00:00:00 2001
From: Pengxuan Zheng <pzheng at quicinc.com>
Date: Fri, 1 Nov 2024 11:41:18 -0700
Subject: [PATCH] [ARM][ConstantIslands] Correct MinNoSplitDisp calculation
MinNoSplitDisp was first introduced in D16890 to handle cases where the
ConstantIslands pass fails to converge in the presence of big basic
blocks. However, the computation of the variable seems to be wrong as it
currently computes the offset immediately following UserBB. In other words, it
represents the distance from the beginning of the function to the end of
UserBB. The distance from the beginning of the function does not seem to be a
good indicator of how big the basic block is unless the basic block is close to
the beginning of the function. I think MinNoSplitDisp should compute the
distance between UserOffset and the end of UserBB instead.
---
llvm/lib/Target/ARM/ARMConstantIslandPass.cpp | 3 +-
.../Thumb2/constant-islands-no-split.mir | 165 ++++++++++++++++++
2 files changed, 167 insertions(+), 1 deletion(-)
create mode 100644 llvm/test/CodeGen/Thumb2/constant-islands-no-split.mir
diff --git a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
index 1312b44b49bdcc..e0575f7c125f67 100644
--- a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
+++ b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -1324,7 +1324,8 @@ bool ARMConstantIslands::findAvailableWater(CPUser &U, unsigned UserOffset,
MachineBasicBlock *UserBB = U.MI->getParent();
BBInfoVector &BBInfo = BBUtils->getBBInfo();
const Align CPEAlign = getCPEAlign(U.CPEMI);
- unsigned MinNoSplitDisp = BBInfo[UserBB->getNumber()].postOffset(CPEAlign);
+ unsigned MinNoSplitDisp =
+ BBInfo[UserBB->getNumber()].postOffset(CPEAlign) - UserOffset;
if (CloserWater && MinNoSplitDisp > U.getMaxDisp() / 2)
return false;
for (water_iterator IP = std::prev(WaterList.end()), B = WaterList.begin();;
diff --git a/llvm/test/CodeGen/Thumb2/constant-islands-no-split.mir b/llvm/test/CodeGen/Thumb2/constant-islands-no-split.mir
new file mode 100644
index 00000000000000..9283ef14ca6cbe
--- /dev/null
+++ b/llvm/test/CodeGen/Thumb2/constant-islands-no-split.mir
@@ -0,0 +1,165 @@
+# RUN: llc -mtriple=thumbv7-linux-gnueabihf -run-pass=arm-cp-islands -arm-constant-island-max-iteration=1 %s -o - | FileCheck %s
+--- |
+ ; ModuleID = 'constant-islands-new-island.ll'
+ source_filename = "constant-islands-new-island.ll"
+ target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+ target triple = "thumbv7-unknown-linux-gnueabihf"
+
+ define void @test(i1 %tst) {
+ entry:
+ %0 = call i32 @llvm.arm.space(i32 2000, i32 undef)
+ br label %smallbb
+
+ smallbb: ; preds = %entry
+ br i1 %tst, label %true, label %false
+
+ true: ; preds = %false, %smallbb
+ %val = phi float [ 1.234500e+04, %smallbb ], [ undef, %false ]
+ %1 = call i32 @llvm.arm.space(i32 2000, i32 undef)
+ call void @bar(float %val)
+ ret void
+
+ false: ; preds = %smallbb
+ br label %true
+ }
+
+ declare void @bar(float)
+
+ ; Function Attrs: nounwind
+ declare i32 @llvm.arm.space(i32 immarg, i32) #0
+
+ attributes #0 = { nounwind }
+
+...
+---
+name: test
+alignment: 2
+exposesReturnsTwice: false
+legalized: false
+regBankSelected: false
+selected: false
+failedISel: false
+tracksRegLiveness: true
+hasWinCFI: false
+noPhis: true
+isSSA: false
+noVRegs: true
+hasFakeUses: false
+callsEHReturn: false
+callsUnwindInit: false
+hasEHCatchret: false
+hasEHScopes: false
+hasEHFunclets: false
+isOutlined: false
+debugInstrRef: false
+failsVerification: false
+tracksDebugUserValues: false
+registers: []
+liveins:
+ - { reg: '$r0', virtual-reg: '' }
+frameInfo:
+ isFrameAddressTaken: false
+ isReturnAddressTaken: false
+ hasStackMap: false
+ hasPatchPoint: false
+ stackSize: 16
+ offsetAdjustment: 0
+ maxAlignment: 4
+ adjustsStack: true
+ hasCalls: true
+ stackProtector: ''
+ functionContext: ''
+ maxCallFrameSize: 0
+ cvBytesOfCalleeSavedRegisters: 0
+ hasOpaqueSPAdjustment: false
+ hasVAStart: false
+ hasMustTailInVarArgFunc: false
+ hasTailCall: false
+ isCalleeSavedInfoValid: true
+ localFrameSize: 0
+ savePoint: ''
+ restorePoint: ''
+fixedStack: []
+stack:
+ - { id: 0, name: '', type: spill-slot, offset: -12, size: 4, alignment: 4,
+ 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: -16, size: 4, alignment: 4,
+ 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: -4, size: 4, alignment: 4,
+ stack-id: default, callee-saved-register: '$lr', callee-saved-restored: false,
+ debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+ - { id: 3, name: '', type: spill-slot, offset: -8, size: 4, alignment: 4,
+ stack-id: default, callee-saved-register: '$r7', callee-saved-restored: true,
+ debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+entry_values: []
+callSites: []
+debugValueSubstitutions: []
+constants:
+ - id: 0
+ value: 'float 1.234500e+04'
+ alignment: 4
+ isTargetSpecific: false
+machineFunctionInfo:
+ isLRSpilled: true
+body: |
+ bb.0.entry:
+ successors: %bb.1(0x80000000)
+ liveins: $r0, $r7, $lr
+
+ frame-setup tPUSH 14 /* CC::al */, $noreg, killed $r7, killed $lr, implicit-def $sp, implicit $sp
+ frame-setup CFI_INSTRUCTION def_cfa_offset 8
+ frame-setup CFI_INSTRUCTION offset $lr, -4
+ frame-setup CFI_INSTRUCTION offset $r7, -8
+ $sp = frame-setup tSUBspi $sp, 2, 14 /* CC::al */, $noreg
+ frame-setup CFI_INSTRUCTION def_cfa_offset 16
+ tSTRspi killed $r0, $sp, 1, 14 /* CC::al */, $noreg :: (store (s32) into %stack.0)
+ renamable $r0 = IMPLICIT_DEF
+ dead renamable $r0 = SPACE 2000, killed renamable $r0
+ t2B %bb.1, 14 /* CC::al */, $noreg
+
+ bb.1.smallbb:
+ successors: %bb.2(0x40000000), %bb.3(0x40000000)
+
+ $r0 = tLDRspi $sp, 1, 14 /* CC::al */, $noreg :: (load (s32) from %stack.0)
+ renamable $s0 = VLDRS %const.0, 0, 14 /* CC::al */, $noreg :: (load (s32) from constant-pool)
+ renamable $r0, dead $cpsr = tLSLri renamable $r0, 31, 14 /* CC::al */, $noreg
+ tCMPi8 killed renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
+ VSTRS killed $s0, $sp, 0, 14 /* CC::al */, $noreg :: (store (s32) into %stack.1)
+ t2Bcc %bb.3, 0 /* CC::eq */, killed $cpsr
+ t2B %bb.2, 14 /* CC::al */, $noreg
+
+ bb.2.true:
+ $s0 = VLDRS $sp, 0, 14 /* CC::al */, $noreg :: (load (s32) from %stack.1)
+ renamable $r0 = IMPLICIT_DEF
+ dead renamable $r0 = SPACE 2000, killed renamable $r0
+ tBL 14 /* CC::al */, $noreg, @bar, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $s0, implicit-def $sp
+ $sp = frame-destroy tADDspi $sp, 2, 14 /* CC::al */, $noreg
+ frame-destroy tPOP_RET 14 /* CC::al */, $noreg, def $r7, def $pc
+
+ bb.3.false:
+ successors: %bb.2(0x80000000)
+
+ renamable $s0 = IMPLICIT_DEF
+ t2B %bb.2, 14 /* CC::al */, $noreg
+
+...
+# Check that smallbb is not split by the constant islands pass. Previously,
+# smallbb was split due to incorrect calculation of MinNoSplitDisp.
+#
+# CHECK: bb.1.smallbb:
+# CHECK-NEXT: successors: %bb.3(0x40000000), %bb.4(0x40000000)
+# CHECK-NEXT: {{^ $}}
+# CHECK-NEXT: $r0 = tLDRspi $sp, 1, 14 /* CC::al */, $noreg :: (load (s32) from %stack.0)
+# CHECK-NEXT: renamable $s0 = VLDRS %const.1, 0, 14 /* CC::al */, $noreg :: (load (s32) from constant-pool)
+# CHECK-NEXT: renamable $r0, dead $cpsr = tLSLri renamable $r0, 31, 14 /* CC::al */, $noreg
+# CHECK-NEXT: tCMPi8 killed renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
+# CHECK-NEXT: VSTRS killed $s0, $sp, 0, 14 /* CC::al */, $noreg :: (store (s32) into %stack.1)
+# CHECK-NEXT: t2Bcc %bb.4, 0 /* CC::eq */, killed $cpsr
+# CHECK-NEXT: tB %bb.3, 14 /* CC::al */, $noreg
+# CHECK-NEXT: {{^ $}}
+# CHECK-NEXT: bb.2 (align 4):
+# CHECK-NEXT: successors:
+# CHECK-NEXT: {{^ $}}
+# CHECK-NEXT: CONSTPOOL_ENTRY 1, %const.0, 4
More information about the llvm-commits
mailing list