Skip to content

Commit 6b5888c

Browse files
authored
[AArch64] Verify ldp/stp alignment stricter (#83948)
When ldp-aligned-only/stp-aligned-only is specified, modified to cancel ldp/stp transformation if MachineMemOperand is not present or the access size is unknown. In the previous implementation, the test passed when there was no MachineMemOperand. Also, if the size was unknown, an incorrect value was used or an assertion failed. (But actually, if there is no MachineMemOperand, it will be excluded from the target by isCandidateToMergeOrPair() before reaching the part.) A statistic NumFailedAlignmentCheck is added. NumPairCreated is modified so that it only counts if it is not cancelled.
1 parent 26058e6 commit 6b5888c

File tree

2 files changed

+81
-19
lines changed

2 files changed

+81
-19
lines changed

llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ STATISTIC(NumUnscaledPairCreated,
6262
"Number of load/store from unscaled generated");
6363
STATISTIC(NumZeroStoresPromoted, "Number of narrow zero stores promoted");
6464
STATISTIC(NumLoadsFromStoresPromoted, "Number of loads from stores promoted");
65+
STATISTIC(NumFailedAlignmentCheck, "Number of load/store pair transformation "
66+
"not passed the alignment check");
6567

6668
DEBUG_COUNTER(RegRenamingCounter, DEBUG_TYPE "-reg-renaming",
6769
"Controls which pairs are considered for renaming");
@@ -2337,9 +2339,6 @@ bool AArch64LoadStoreOpt::tryToPairLdStInst(MachineBasicBlock::iterator &MBBI) {
23372339
MachineBasicBlock::iterator Paired =
23382340
findMatchingInsn(MBBI, Flags, LdStLimit, /* FindNarrowMerge = */ false);
23392341
if (Paired != E) {
2340-
++NumPairCreated;
2341-
if (TII->hasUnscaledLdStOffset(MI))
2342-
++NumUnscaledPairCreated;
23432342
// Keeping the iterator straight is a pain, so we let the merge routine tell
23442343
// us what the next instruction is after it's done mucking about.
23452344
auto Prev = std::prev(MBBI);
@@ -2349,31 +2348,38 @@ bool AArch64LoadStoreOpt::tryToPairLdStInst(MachineBasicBlock::iterator &MBBI) {
23492348
MachineMemOperand *MemOp =
23502349
MI.memoperands_empty() ? nullptr : MI.memoperands().front();
23512350

2352-
// Get the needed alignments to check them if
2353-
// ldp-aligned-only/stp-aligned-only features are opted.
2354-
uint64_t MemAlignment = MemOp ? MemOp->getAlign().value() : -1;
2355-
uint64_t TypeAlignment = MemOp ? Align(MemOp->getSize()).value() : -1;
2351+
// If a load/store arrives and ldp/stp-aligned-only feature is opted, check
2352+
// that the alignment of the source pointer is at least double the alignment
2353+
// of the type.
2354+
if ((MI.mayLoad() && Subtarget->hasLdpAlignedOnly()) ||
2355+
(MI.mayStore() && Subtarget->hasStpAlignedOnly())) {
2356+
// If there is no size/align information, cancel the transformation.
2357+
if (!MemOp || !MemOp->getMemoryType().isValid()) {
2358+
NumFailedAlignmentCheck++;
2359+
return false;
2360+
}
23562361

2357-
// If a load arrives and ldp-aligned-only feature is opted, check that the
2358-
// alignment of the source pointer is at least double the alignment of the
2359-
// type.
2360-
if (MI.mayLoad() && Subtarget->hasLdpAlignedOnly() && MemOp &&
2361-
MemAlignment < 2 * TypeAlignment)
2362-
return false;
2362+
// Get the needed alignments to check them if
2363+
// ldp-aligned-only/stp-aligned-only features are opted.
2364+
uint64_t MemAlignment = MemOp->getAlign().value();
2365+
uint64_t TypeAlignment = Align(MemOp->getSize()).value();
23632366

2364-
// If a store arrives and stp-aligned-only feature is opted, check that the
2365-
// alignment of the source pointer is at least double the alignment of the
2366-
// type.
2367-
if (MI.mayStore() && Subtarget->hasStpAlignedOnly() && MemOp &&
2368-
MemAlignment < 2 * TypeAlignment)
2369-
return false;
2367+
if (MemAlignment < 2 * TypeAlignment) {
2368+
NumFailedAlignmentCheck++;
2369+
return false;
2370+
}
2371+
}
23702372

23712373
MBBI = mergePairedInsns(MBBI, Paired, Flags);
23722374
// Collect liveness info for instructions between Prev and the new position
23732375
// MBBI.
23742376
for (auto I = std::next(Prev); I != MBBI; I++)
23752377
updateDefinedRegisters(*I, DefinedInBB, TRI);
23762378

2379+
++NumPairCreated;
2380+
if (TII->hasUnscaledLdStOffset(MI))
2381+
++NumUnscaledPairCreated;
2382+
23772383
return true;
23782384
}
23792385
return false;
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
2+
# RUN: llc -O2 -mtriple=aarch64 -mcpu=ampere1 -simplify-mir -o - %s -run-pass=aarch64-ldst-opt | FileCheck %s --check-prefixes=CHECK
3+
# RUN: llc -O2 -mtriple=aarch64 -simplify-mir -o - %s -run-pass=aarch64-ldst-opt | FileCheck %s --check-prefixes=CHECK-DEFAULT
4+
5+
--- |
6+
define i32 @ldp_no_size_info(ptr %0) #0 {
7+
%2 = ptrtoint ptr %0 to i64
8+
%3 = and i64 %2, -64
9+
%4 = inttoptr i64 %3 to ptr
10+
%5 = load i32, ptr %4, align 4
11+
%6 = getelementptr inbounds i32, ptr %4, i64 1
12+
%7 = load i32, ptr %6, align 4
13+
%8 = add nsw i32 %7, %5
14+
ret i32 %8
15+
}
16+
17+
...
18+
---
19+
name: ldp_no_size_info
20+
alignment: 64
21+
tracksRegLiveness: true
22+
tracksDebugUserValues: true
23+
liveins:
24+
- { reg: '$x0' }
25+
frameInfo:
26+
maxAlignment: 1
27+
maxCallFrameSize: 0
28+
machineFunctionInfo:
29+
hasRedZone: false
30+
body: |
31+
bb.0 (%ir-block.1):
32+
liveins: $x0
33+
34+
; CHECK-LABEL: name: ldp_no_size_info
35+
; CHECK: liveins: $x0
36+
; CHECK-NEXT: {{ $}}
37+
; CHECK-NEXT: renamable $x8 = ANDXri killed renamable $x0, 7865
38+
; CHECK-NEXT: renamable $w9 = LDRWui renamable $x8, 0 :: (load unknown-size from %ir.4, align 1)
39+
; CHECK-NEXT: renamable $w8 = LDRWui killed renamable $x8, 1 :: (load unknown-size from %ir.6, align 1)
40+
; CHECK-NEXT: $w0 = ADDWrs killed renamable $w8, killed renamable $w9, 0
41+
; CHECK-NEXT: RET undef $lr, implicit $w0
42+
;
43+
; CHECK-DEFAULT-LABEL: name: ldp_no_size_info
44+
; CHECK-DEFAULT: liveins: $x0
45+
; CHECK-DEFAULT-NEXT: {{ $}}
46+
; CHECK-DEFAULT-NEXT: renamable $x8 = ANDXri killed renamable $x0, 7865
47+
; CHECK-DEFAULT-NEXT: renamable $w9, renamable $w8 = LDPWi renamable $x8, 0 :: (load unknown-size from %ir.4, align 1), (load unknown-size from %ir.6, align 1)
48+
; CHECK-DEFAULT-NEXT: $w0 = ADDWrs killed renamable $w8, killed renamable $w9, 0
49+
; CHECK-DEFAULT-NEXT: RET undef $lr, implicit $w0
50+
renamable $x8 = ANDXri killed renamable $x0, 7865
51+
renamable $w9 = LDRWui renamable $x8, 0 :: (load unknown-size from %ir.4)
52+
renamable $w8 = LDRWui killed renamable $x8, 1 :: (load unknown-size from %ir.6)
53+
$w0 = ADDWrs killed renamable $w8, killed renamable $w9, 0
54+
RET undef $lr, implicit $w0
55+
56+
...

0 commit comments

Comments
 (0)