Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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

This file was deleted.

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
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
4 changes: 0 additions & 4 deletions llvm/tools/llvm-exegesis/lib/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
include_directories(
${LLVM_MAIN_SRC_DIR}/lib/Target/AArch64
${LLVM_BINARY_DIR}/lib/Target/AArch64
)

set(LLVM_EXEGESIS_TARGETS)
if (LLVM_TARGETS_TO_BUILD MATCHES "X86")
Expand Down
57 changes: 16 additions & 41 deletions llvm/tools/llvm-exegesis/lib/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Error.h"
#include "llvm/TargetParser/SubtargetFeature.h"
#include "AArch64.h"
#include "AArch64RegisterInfo.h"

namespace llvm {
namespace exegesis {
Expand All @@ -37,49 +35,26 @@ const ExegesisTarget *ExegesisTarget::lookup(Triple TT) {
return nullptr;
}

static bool isPointerAuthOpcode(unsigned Opcode) {
switch (Opcode) {
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;
default:
return false;
}
}

static bool isUncheckedAccessOpcode(unsigned Opcode) {
switch (Opcode) {
case AArch64::LDGM:
return true;
default:
return false;
}
}

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 (isPointerAuthOpcode(Opcode) || isUncheckedAccessOpcode(Opcode))
return "Unsupported opcode: isPointerAuth/isUncheckedAccess";
if (InstrDesc.isPseudo())
Copy link
Contributor

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

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

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
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

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

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