Skip to content

[AArch64] Mark Armv8.4-a LDAPUR* instructions as mayLoad #171142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
c-rhodes wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[AArch64] Mark Armv8.4-a LDAPUR* instructions as mayLoad #171142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/lib/Target/AArch64/AArch64InstrFormats.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4384,6 +4384,7 @@ class BaseLoadStoreUnscale<bits<2> sz, bit V, bits<2> opc, dag oops, dag iops,
// Armv8.4 LDAPR & STLR with Immediate Offset instruction
multiclass BaseLoadUnscaleV84<string asm, bits<2> sz, bits<2> opc,
DAGOperand regtype > {
let mayLoad = 1 in
def i : BaseLoadStoreUnscale<sz, 0, opc, (outs regtype:$Rt),
(ins GPR64sp:$Rn, simm9:$offset), asm, []>,
Sched<[WriteST]> {
Copy link
Collaborator

@davemgreen davemgreen Dec 8, 2025

Choose a reason for hiding this comment

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

This should probably be WriteLD or WriteAtomic. The scheduling info in the tests looks incorrect.

Copy link
Collaborator Author

@c-rhodes c-rhodes Dec 8, 2025

Choose a reason for hiding this comment

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

Good spot! I can post a separate PR.

FWIW I've not been paying close attention to the scheduling info with recent patches tbh, just trying to refactor the tests and increase coverage but i can believe there's many with bad info such as this.

Copy link
Collaborator Author

@c-rhodes c-rhodes Dec 10, 2025

Choose a reason for hiding this comment

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

posted a fix for this #171637

Expand Down
10 changes: 5 additions & 5 deletions llvm/test/tools/llvm-mca/AArch64/Neoverse/N2-rcpc-immo-instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
# CHECK-NEXT: 2 1 0.50 * ldapur x20, [x13]
# CHECK-NEXT: 2 1 0.50 * ldapurb w13, [x17]
# CHECK-NEXT: 2 1 0.50 * ldapurh w3, [x22]
# CHECK-NEXT: 2 1 0.50 U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 * U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 * U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 * U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 * U ldapursw x3, [x18]
Comment on lines +17 to +21
Copy link
Contributor

@cofibrant cofibrant Dec 8, 2025

Choose a reason for hiding this comment

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

I'm surprised these are still marked as 'has side effects'. Could you explain what the side effects are?

Copy link
Collaborator Author

@c-rhodes c-rhodes Dec 8, 2025

Choose a reason for hiding this comment

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

yeah tbh im not sure about that either, i can't see why they shouldn't be marked as mayLoad so I suspect that was just an oversight, but I didnt add these instructions

Copy link
Contributor

@cofibrant cofibrant Dec 8, 2025

Choose a reason for hiding this comment

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

The context is that I spotted that Apple clang thinks these instructions may load and do not have side effects and I wanted to understand which implementation is correct (if either) and fix accordingly

Copy link
Collaborator Author

@c-rhodes c-rhodes Dec 8, 2025

Choose a reason for hiding this comment

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

I'm not familiar with the semantics of these instructions, but from a quick glance at https://developer.arm.com/documentation/102336/0100/Load-Acquire-and-Store-Release-instructions it looks like they have implicit barrier semantics, so while I can't give a definitive answer I would err more towards them having side-effects than not. dmb instructions do, and these have less constrained but similar semantics it seems.

Copy link
Contributor

@cofibrant cofibrant Dec 8, 2025

Choose a reason for hiding this comment

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

Interesting, thanks!

# CHECK-NEXT: 2 1 0.50 * stlur w3, [x27]
# CHECK-NEXT: 2 1 0.50 * stlur x23, [x25]
# CHECK-NEXT: 2 1 0.50 * stlurb w30, [x17]
Expand Down
10 changes: 5 additions & 5 deletions llvm/test/tools/llvm-mca/AArch64/Neoverse/N3-rcpc-immo-instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
# CHECK-NEXT: 2 1 0.50 * ldapur x20, [x13]
# CHECK-NEXT: 2 1 0.50 * ldapurb w13, [x17]
# CHECK-NEXT: 2 1 0.50 * ldapurh w3, [x22]
# CHECK-NEXT: 2 1 0.50 U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 * U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 * U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 * U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 * U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * stlur w3, [x27]
# CHECK-NEXT: 2 1 0.50 * stlur x23, [x25]
# CHECK-NEXT: 2 1 0.50 * stlurb w30, [x17]
Expand Down
10 changes: 5 additions & 5 deletions llvm/test/tools/llvm-mca/AArch64/Neoverse/V1-rcpc-immo-instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
# CHECK-NEXT: 2 1 0.50 * ldapur x20, [x13]
# CHECK-NEXT: 2 1 0.50 * ldapurb w13, [x17]
# CHECK-NEXT: 2 1 0.50 * ldapurh w3, [x22]
# CHECK-NEXT: 2 1 0.50 U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 * U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 * U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 * U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 * U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * stlur w3, [x27]
# CHECK-NEXT: 2 1 0.50 * stlur x23, [x25]
# CHECK-NEXT: 2 1 0.50 * stlurb w30, [x17]
Expand Down
10 changes: 5 additions & 5 deletions llvm/test/tools/llvm-mca/AArch64/Neoverse/V2-rcpc-immo-instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
# CHECK-NEXT: 2 1 0.50 * ldapur x20, [x13]
# CHECK-NEXT: 2 1 0.50 * ldapurb w13, [x17]
# CHECK-NEXT: 2 1 0.50 * ldapurh w3, [x22]
# CHECK-NEXT: 2 1 0.50 U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 * U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 * U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 * U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 * U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * stlur w3, [x27]
# CHECK-NEXT: 2 1 0.50 * stlur x23, [x25]
# CHECK-NEXT: 2 1 0.50 * stlurb w30, [x17]
Expand Down
10 changes: 5 additions & 5 deletions llvm/test/tools/llvm-mca/AArch64/Neoverse/V3-rcpc-immo-instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
# CHECK-NEXT: 2 1 0.50 * ldapur x20, [x13]
# CHECK-NEXT: 2 1 0.50 * ldapurb w13, [x17]
# CHECK-NEXT: 2 1 0.50 * ldapurh w3, [x22]
# CHECK-NEXT: 2 1 0.50 U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 * U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 * U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 * U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 * U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * stlur w3, [x27]
# CHECK-NEXT: 2 1 0.50 * stlur x23, [x25]
# CHECK-NEXT: 2 1 0.50 * stlurb w30, [x17]
Expand Down
10 changes: 5 additions & 5 deletions llvm/test/tools/llvm-mca/AArch64/Neoverse/V3AE-rcpc-immo-instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
# CHECK-NEXT: 2 1 0.50 * ldapur x20, [x13]
# CHECK-NEXT: 2 1 0.50 * ldapurb w13, [x17]
# CHECK-NEXT: 2 1 0.50 * ldapurh w3, [x22]
# CHECK-NEXT: 2 1 0.50 U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * U ldapursb w7, [x8]
# CHECK-NEXT: 2 1 0.50 * U ldapursb x29, [x7]
# CHECK-NEXT: 2 1 0.50 * U ldapursh w17, [x19]
# CHECK-NEXT: 2 1 0.50 * U ldapursh x3, [x3]
# CHECK-NEXT: 2 1 0.50 * U ldapursw x3, [x18]
# CHECK-NEXT: 2 1 0.50 * stlur w3, [x27]
# CHECK-NEXT: 2 1 0.50 * stlur x23, [x25]
# CHECK-NEXT: 2 1 0.50 * stlurb w30, [x17]
Expand Down
Loading