Skip to content

[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instructions #132346

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

Merged
madhur13490 merged 11 commits into from
Apr 9, 2025
Merged

[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instructions #132346

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f1e1d7a
[llvm-exegesis] [AArch64] Resolving Illegal Instruction Error
lakshayk-nv Mar 21, 2025
5039c2b
[llvm-exegesis] [AArch64] Use enum values for opcode validation
lakshayk-nv Mar 24, 2025
6b81e65
[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instru…
lakshayk-nv Mar 26, 2025
bef8e21
Merge branch 'main' of https://github.com/llvm/llvm-project into llvm…
lakshayk-nv Apr 1, 2025
dade502
[llvm-exegesis][AArch64] Formatting changes
lakshayk-nv Apr 1, 2025
b6ce448
[llvm-exegesis] Add Linux PR_PAC_* constants and update opcode handling
lakshayk-nv Apr 1, 2025
783f86d
[llvm-exegesis][AArch64] Updated the testcase for AUT instruction class.
lakshayk-nv Apr 1, 2025
70eb3bf
[llvm-exegesis][AArch64] Refactor opcode handling for pointer authent…
lakshayk-nv Apr 3, 2025
a1fedb4
[llvm-exegesis][AArch64] Enhance pointer authentication handling for …
lakshayk-nv Apr 3, 2025
06fbe30
Revert "[llvm-exegesis][AArch64] Formatting changes"
lakshayk-nv Apr 4, 2025
3af87a0
[llvm-exegesis][AArch64] Revert back unnecessary unfolded error and f…
lakshayk-nv Apr 7, 2025
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
8 changes: 8 additions & 0 deletions llvm/test/tools/llvm-exegesis/AArch64/skip_unsupported_instructions.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# REQUIRES: aarch64-registered-target

# Check for skipping of illegal instruction errors (AUT and LDGM)
# RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --opcode-name=AUTIA --benchmark-phase=assemble-measured-code 2>&1 | FileCheck %s --check-prefix=CHECK-AUTIA
# CHECK-AUTIA: AUTIA: Unsupported opcode: isPointerAuth

# RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --opcode-name=LDGM --benchmark-phase=assemble-measured-code 2>&1 | FileCheck %s --check-prefix=CHECK-LDGM
# CHECK-LDGM: LDGM: Unsupported opcode: load tag multiple
38 changes: 38 additions & 0 deletions llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,5 +147,43 @@ void InitializeAArch64ExegesisTarget() {
ExegesisTarget::registerTarget(getTheExegesisAArch64Target());
}

bool isPointerAuth(unsigned Opcode) {
switch (Opcode) {
default:
return false;

// FIXME: Pointer Authentication instructions.
// We would like to measure these instructions, but they can behave differently on
// different platforms, and maybe the snippets need to look different for these instructions,
// so let's disable this for now.
case AArch64::AUTDA:
case AArch64::AUTDB:
case AArch64::AUTDZA:
case AArch64::AUTDZB:
case AArch64::AUTIA:
case AArch64::AUTIA1716:
case AArch64::AUTIASP:
case AArch64::AUTIAZ:
case AArch64::AUTIB:
case AArch64::AUTIB1716:
case AArch64::AUTIBSP:
case AArch64::AUTIBZ:
case AArch64::AUTIZA:
case AArch64::AUTIZB:
return true;
}
}

bool isLoadTagMultiple(unsigned Opcode) {
switch (Opcode) {
default:
return false;

// Load tag multiple instruction
case AArch64::LDGM:
return true;
}
}

} // namespace exegesis
} // namespace llvm
22 changes: 16 additions & 6 deletions llvm/tools/llvm-exegesis/lib/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,22 @@ const char *
ExegesisTarget::getIgnoredOpcodeReasonOrNull(const LLVMState &State,
unsigned Opcode) const {
const MCInstrDesc &InstrDesc = State.getIC().getInstr(Opcode).Description;
if (InstrDesc.isPseudo() || InstrDesc.usesCustomInsertionHook())
return "Unsupported opcode: isPseudo/usesCustomInserter";
if (InstrDesc.isBranch() || InstrDesc.isIndirectBranch())
return "Unsupported opcode: isBranch/isIndirectBranch";
if (InstrDesc.isCall() || InstrDesc.isReturn())
return "Unsupported opcode: isCall/isReturn";
if (InstrDesc.isPseudo())
Copy link
Contributor

@boomanaiden154 boomanaiden154 Apr 3, 2025

Choose a reason for hiding this comment

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

All the changes in this file seem to be formatting changes that are now irrelevant to the PR? They should probably be reverted.

Copy link
Contributor Author

@lakshayk-nv lakshayk-nv Apr 4, 2025

Choose a reason for hiding this comment

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

split up the first couple of if-statements as I don't like error messages that say "it can be this or that":

Sjoerd suggested 'unclubbing' the unsupported error message to specify the exact reason. Thus, keeping this and formatter's changes for this file only.

Copy link
Contributor

@boomanaiden154 boomanaiden154 Apr 4, 2025

Choose a reason for hiding this comment

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

I would prefer these changes go in a separate patch.

return "Unsupported opcode: isPseudo";
if ( InstrDesc.usesCustomInsertionHook())
return "Unsupported opcode: usesCustomInserter";
if (InstrDesc.isBranch())
return "Unsupported opcode: isBranch";
if (InstrDesc.isIndirectBranch())
return "Unsupported opcode: isIndirectBranch";
if (InstrDesc.isCall())
return "Unsupported opcode: isCall";
if (InstrDesc.isReturn())
return "Unsupported opcode: isReturn";
if (isPointerAuth(Opcode))
return "Unsupported opcode: isPointerAuth";
if (isLoadTagMultiple(Opcode))
return "Unsupported opcode: load tag multiple";
return nullptr;
}

Expand Down
3 changes: 3 additions & 0 deletions llvm/tools/llvm-exegesis/lib/Target.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ class ExegesisTarget {
const OpcodeAvailabilityChecker IsOpcodeAvailable;
};

bool isPointerAuth(unsigned Opcode);
Copy link
Contributor

@boomanaiden154 boomanaiden154 Apr 3, 2025

Choose a reason for hiding this comment

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

These do not make sense to have in the normal target given they are specific to AArch64. It would be better (in my opinion) to add a method similar to getIgnoredOpcodeReasonOrNull that can be overrided by targets and then gets invoked in getIgnoredOpcodeReasonsOrNull.

Copy link
Contributor Author

@lakshayk-nv lakshayk-nv Apr 3, 2025

Choose a reason for hiding this comment

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

Yes, That implementation was bad.
Updated implementation, Now overriding getIgnoredOpcodeReasonOrNull() , which checks isPointerAuth() and isLoadTagMultiple() additionally for AArch64.

Thanks :)

bool isLoadTagMultiple(unsigned Opcode);

} // namespace exegesis
} // namespace llvm

Expand Down
Loading