Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# REQUIRES: aarch64-registered-target
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick about the filename err_skip_illegal_Instruction.s: they are not illegal instructions per se, it's more that they are unsupported (at the moment), and also one word is capitalised and the other aren't so is a bit inconsistent. My suggestion would be skip_unsupported_instructions.s or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally named it err_skip_illegal_Instruction.s to indicate it's a test case for error skipping behavior, but you're right that skip_unsupported_instructions.s would be more accurate since these instructions are unsupported rather than illegal. I will push required changes.

Btw, Can you weigh in for checking other skipped errors (isPseudo, isReturn etc.).
Does it make sense to check them also in this testfile, (just for completeness) ?


# 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
# CHECK: AUTIA: Unsupported opcode: isPointerAuth/isUncheckedAccess

# RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --opcode-name=LDGM --benchmark-phase=assemble-measured-code 2>&1
# CHECK: LDGM: Unsupported opcode: isPointerAuth/isUncheckedAccess
4 changes: 4 additions & 0 deletions llvm/tools/llvm-exegesis/lib/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
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
35 changes: 35 additions & 0 deletions llvm/tools/llvm-exegesis/lib/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#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 @@ -35,6 +37,37 @@ const ExegesisTarget *ExegesisTarget::lookup(Triple TT) {
return nullptr;
}

static bool isPointerAuthOpcode(unsigned Opcode) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another nitpick, I wanted to suggest that we create one isUnsupportedOpcode() function that could use these new helper functions, but then realised that is function getIgnoredOpcodeReasonOrNull(). We might as well use that function then, here would be my suggestion and as you see I also split up the first couple of if-statements as I don't like error messages that say "it can be this or that":

    const char *
    ExegesisTarget::getIgnoredOpcodeReasonOrNull(const LLVMState &State,
                                             unsigned Opcode) const {
        const MCInstrDesc &InstrDesc = State.getIC().getInstr(Opcode).Description;

        if (InstrDesc.isPseudo())
          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";

         switch (Opcode) {
         default:
           return nullptr;

         // 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 ""Unsupported opcode: isPointerAuth";

         // Load tag multiple instruction
         case AArch64::LDGM:
           return "Unsupported opcode: load tag multiple";
       }
       return nullptr;
    }

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 {
Expand All @@ -45,6 +78,8 @@ ExegesisTarget::getIgnoredOpcodeReasonOrNull(const LLVMState &State,
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";
return nullptr;
}

Expand Down
Loading