[llvm-exegesis] [AArch64] Resolving "not all operands are initialized by snippet generator" #142529

lakshayk-nv · 2025-06-03T05:47:24Z

Exegesis for AArch64 arch, before this patch only handles/initialize Immediate and Register Operands.
For opcodes requiring rest operand types exegesis exits with Error: "not all operands are initialized by snippet generator".

Thus, to resolve a given error we have to initialize required operand types.
i.e., For "not all operands are initialized by snippet generator" init OPERAND_UNKNOWN, OPERAND_PCREL,
And For "targets with target-specific operands should implement this" init "OPERAND_FIRST_TARGET"

Implementing randomizeTargetMCOperand() for AArch64 that omitting OPERAND_UNKNOWN and OPERAND_PCREL to an immediate value based on opcode.
PS: Omitting OPERAND_FIRST_TARGET similarly, It was low hanging change thus added in this PR only.

For any future operand type of AArch64 if not initialized will exit with error "Unimplemented operand type: MCOI::OperandType:<#Number>"

FIXME: Init operands forMRS, MSR, MSRpstatesvcrImm1, SYSLxt, SYSxt, UDF
This patch in --mode=inverse_throughput for opcodes MRS, MSR, MSRpstatesvcrImm1, SYSLxt, SYSxt, UDF exits with handled error of snippet crashed while running: Illegal instruction, they previously used to exits with error not all operands initialized by snippet generator.

This patch doesn't regress any working opcode. And valid initialization of operands for those 6 opcodes will be in upcoming PRs.

[For completeness] Additionally, exegesis beforehand and with this patch too, throws illegal instruction in throughput mode, for these opcodes too (APAS, DCPS1, DCPS2, DCPS3, HLT, HVC, SMC, STGM, STZGM). Will look into them later.

Remove aarch64 guard in SnippetGenerator.cpp.

Please review: @sjoerdmeijer, @boomanaiden154, @davemgreen

… by the snippet generator" by omit OPERAND_UNKNOWN to Immediate

…nippet generation, omiting immediate valued 0.

…ARGET.

…r omitted opcode type.

…tMCOperand.

…move out of scope operand type.

llvmbot · 2025-06-03T05:48:01Z

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Lakshay Kumar (lakshayk-nv)

Changes

Implementing randomizeTargetMCOperand() for AArch64 that omitting OPERAND_UNKNOWN and OPERAND_PCREL to an immediate value based on opcode.

Full diff: https://github.com/llvm/llvm-project/pull/142529.diff

2 Files Affected:

(modified) llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp (+38)
(modified) llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp (+8)

diff --git a/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp b/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
index a1eb5a46f21fc..285d888770a53 100644
--- a/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
@@ -162,6 +162,10 @@ class ExegesisAArch64Target : public ExegesisTarget {
   ExegesisAArch64Target()
       : ExegesisTarget(AArch64CpuPfmCounters, AArch64_MC::isOpcodeAvailable) {}
 
+  Error randomizeTargetMCOperand(
+      const Instruction &Instr, const Variable &Var, MCOperand &AssignedValue,
+      const BitVector &ForbiddenRegs) const override;
+
 private:
   std::vector<MCInst> setRegTo(const MCSubtargetInfo &STI, MCRegister Reg,
                                const APInt &Value) const override {
@@ -229,6 +233,40 @@ class ExegesisAArch64Target : public ExegesisTarget {
   }
 };
 
+Error ExegesisAArch64Target::randomizeTargetMCOperand(
+    const Instruction &Instr, const Variable &Var, MCOperand &AssignedValue,
+    const BitVector &ForbiddenRegs) const {
+  const Operand &Op = Instr.getPrimaryOperand(Var);
+  const auto OperandType = Op.getExplicitOperandInfo().OperandType;
+  // Introducing some illegal instructions for (15) a few opcodes
+  // TODO: Look into immediate values to be opcode specific
+  switch (OperandType) {
+    case MCOI::OperandType::OPERAND_UNKNOWN: {
+      unsigned Opcode = Instr.getOpcode();
+      switch (Opcode) {
+        case AArch64::MOVIv2s_msl:
+        case AArch64::MOVIv4s_msl:
+        case AArch64::MVNIv2s_msl:
+        case AArch64::MVNIv4s_msl:
+          AssignedValue = MCOperand::createImm(8); // or 16, as needed
+          return Error::success();
+        default:
+          AssignedValue = MCOperand::createImm(0);
+          return Error::success();
+      }
+    }
+    case MCOI::OperandType::OPERAND_PCREL:
+      AssignedValue = MCOperand::createImm(0);
+      return Error::success();
+    default:
+      break;
+  }
+
+  return make_error<Failure>(
+      Twine("Unimplemented operand type: MCOI::OperandType:")
+          .concat(Twine(static_cast<int>(OperandType))));
+}
+
 } // namespace
 
 static ExegesisTarget *getTheExegesisAArch64Target() {
diff --git a/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp b/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
index 04064ae1d8441..d4381c3b123f0 100644
--- a/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
@@ -276,6 +276,14 @@ static Error randomizeMCOperand(const LLVMState &State,
     AssignedValue = MCOperand::createReg(randomBit(AllowedRegs));
     break;
   }
+  /// Omit unknown and pc-relative operands to imm value based on the instruction
+  // TODO: Neccesity of AArch64 guard ?
+#ifdef __aarch64__
+  case MCOI::OperandType::OPERAND_UNKNOWN:
+  case MCOI::OperandType::OPERAND_PCREL:
+    return State.getExegesisTarget().randomizeTargetMCOperand(
+        Instr, Var, AssignedValue, ForbiddenRegs);
+#endif
   default:
     break;
   }

github-actions · 2025-06-03T05:49:40Z

✅ With the latest revision this PR passed the C/C++ code formatter.

boomanaiden154

This also needs a test.

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

davemgreen · 2025-06-03T08:04:00Z

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

+        case AArch64::MOVIv4s_msl:
+        case AArch64::MVNIv2s_msl:
+        case AArch64::MVNIv4s_msl:
+          AssignedValue = MCOperand::createImm(8); // or 16, as needed


Ideally this would be marked as a OPERAND_IMMEDIATE with a range that tablegen understood, so that it could be used for validation of codegen and reused here.

Yes, If i understood this correctly Ideally tablegen file can be updated.
Currently, the generated .inc file have OPERAND_UNKNOWN for opcode that thrown uninit operands.
Omission works out to get us benchmarking measurements for opcodes throwing this error.

As expanded on later comments, Introduced two new operand kinds OPERAND_MSL_SHIFT_2S and OPERAND_MSL_SHIFT_4S in AArch64 backend.

Thanks!

…ndomizeTargetMCOperand to omit to Immediate

… omittion to resolve illegal instruction

lakshayk-nv · 2025-06-12T07:20:34Z

Resolving: targets with target-specific operands should implement this
Low hanging, Omission OPERAND_FIRST_TARGET with Immediate of value 0.
[Effect]
Support added for ASIMD UMOV, SMOV INDEX_RR_H, and few Load instructions.

INDEX_RR_H,
UMOVvi8_idx0,UMOVvi16_idx0,UMOVvi32_idx0,UMOVvi64_idx0,
SMOVvi8to64_idx0,SMOVvi8to32_idx0,SMOVvi32to64_idx0,SMOVvi16to64_idx0,SMOVvi16to32_idx0

Support added and expectedly segfault appears
LD2W,LD1Onev16b,LD2Twov16b,LD4Fourv4h,LDPSi,LD2Rv4s,LD4Rv2s

sjoerdmeijer · 2025-06-13T12:27:42Z

I would also like to see a test case.
Or is that difficult because this is randomising the registers? But from a quick look, we are not really randomly assigning values, always 0, so we could easily add test cases?

… error-resolution.s

boomanaiden154 · 2025-06-16T17:24:11Z

Or is that difficult because this is randomising the registers?

Even if we are randomizing registers, a regex of some form would probably suffice.

lakshayk-nv · 2025-06-19T16:37:13Z

Added the testcase for the patch for each operand type and mode (latency and throughput).
Thanks!

sjoerdmeijer

I think this is forward progress as it now is able to successfully generate and execute more snippets and handle more cases, so looks good to me from that point of view, and I am okay that are limitations and TODOs. However, before we merge this, I would like to see more crisps descriptions and comments, so that we understand better the limitations of the current work and what we would need to do next.

The description of this patch isn't clear to me, it doesn't parse for me:

Implementing randomizeTargetMCOperand() for AArch64 that omitting OPERAND_UNKNOWN and OPERAND_PCREL to an immediate value based on opcode.

I don't understand "omitting" here, so please clarify. The way I understand it at this moment, I think you mean to say that you now actually handle these operand types, but I could be wrong.

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

…d type in randomizeTargetMCOperand

abhilash1910

LGTM modulo other reviewer's comments

…rands

…_TYPES i.e. OPERAND_MSL_SHIFT_4S and OPERAND_MSL_SHIFT_2S

davemgreen

Thanks for adding this, I think it will be useful for adding verifier checks too.

llvm/lib/Target/AArch64/AArch64InstrFormats.td

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

llvm/lib/Target/AArch64/AArch64InstrFormats.td

…odes

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

davemgreen · 2025-08-20T09:08:10Z

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp

+      // Creates 2-element 32-bit vector with 8-bit imm at positions [15:8] &
+      // [47:40] Shift value 264 (0x108) for Type 7 pattern encoding Corresponds
+      // to AArch64_AM::encodeAdvSIMDModImmType7()
+      AssignedValue = MCOperand::createImm(264);


Why use 264 for one and 272 for the other? (And does the immediate matter for scheduling?)

Am I right that both MOVIv2s_msl and MOVIv4s_msl accept both immediates, depending on the shift amount. We might as well just pick one for both I think. Maybe in the comment say that it can be 264 (msl #8) or 272 (msl #16), so we pick 264.

Sure, For simplicity we can use 264 (msl#8). As, there is no change in instruction characteristics (inverse_throughput), just the underlying generated assembly from msl #16 to msl #8

davemgreen · 2025-08-20T09:09:00Z

llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp

@@ -276,6 +276,12 @@ static Error randomizeMCOperand(const LLVMState &State,
    AssignedValue = MCOperand::createReg(randomBit(AllowedRegs));
    break;
  }
+  /// Omit unknown and pc-relative operands to imm value based on the
+  /// instruction
+  case MCOI::OperandType::OPERAND_UNKNOWN:


This should be removed too I think.

This is helps us to get measurement values for 1000+ opcode. We have sanity checked this by comparing the measurements values reported by exegesis and (latency and inverse-throughput) for them. And there are few opcodes for which omitting immediate with value 0 is incorrect, They are documented in code and this PR (like UDF, `SYSLxt)

davemgreen · 2025-08-20T09:13:20Z

llvm/lib/Target/AArch64/AArch64InstrFormats.td

@@ -1319,7 +1319,9 @@ def logical_vec_hw_shift : Operand<i32> {
 // A vector move shifter operand:
 //  {0} - imm1: #8 or #16
 def move_vec_shift : Operand<i32> {
+  let OperandNamespace = "AArch64";


If this is needed put it next to OperandType and maybe put them both at the bottom.

Moved definition below OPERAND_IMMEDIATE. Please confirm if this is what is meant by you.
Thanks!

llvm/lib/Target/AArch64/AArch64InstrFormats.td

…ft to be msl llvm#8

lakshayk-nv added 6 commits May 30, 2025 06:43

[llvm-exegesis] [AArch64] Resolve " Not all operands were initialized…

b1919dd

… by the snippet generator" by omit OPERAND_UNKNOWN to Immediate

[llvm-exegesis] [AArch64] Include OPERAND_PCREL operand handling in s…

af68e0f

…nippet generation, omiting immediate valued 0.

[llvm-exegesis] [AArch64] WIP. Introduce handling for OPERAND_FIRST_T…

5697760

…ARGET.

[llvm-exegesis] [AArch64] Explore opcode-specific immediate values fo…

75c2e65

…r omitted opcode type.

[llvm-exegesis] [AArch64] Refactor operand handling in randomizeTarge…

9d425dc

…tMCOperand.

[llvm-exegesis] [AArch64] Update comments for operand handling and re…

9a1feb2

…move out of scope operand type.

llvmbot added the tools:llvm-exegesis label Jun 3, 2025

RKSimon requested a review from boomanaiden154 June 3, 2025 07:01

boomanaiden154 reviewed Jun 3, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Outdated Show resolved Hide resolved

llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp Outdated Show resolved Hide resolved

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Outdated Show resolved Hide resolved

[llvm-exegesis] [AArch64] Format changes.

f567870

davemgreen reviewed Jun 3, 2025

View reviewed changes

davemgreen requested a review from sjoerdmeijer June 3, 2025 08:08

lakshayk-nv added 3 commits June 3, 2025 07:35

[llvm-exegesis] [AArch64] Remove unneccessary AArch64 guard

66fdd39

[llvm-exegesis] [AArch64] Add handling for OPERAND_FIRST_TARGET in ra…

59452e5

…ndomizeTargetMCOperand to omit to Immediate

[llvm-exegesis] [AArch64] Documenting opcodes requiring some specific…

1726bea

… omittion to resolve illegal instruction

lakshayk-nv requested review from davemgreen and boomanaiden154 June 13, 2025 18:21

[llvm-exegesis] [AArch64] Add tests for operand omission scenarios in…

c26aa00

… error-resolution.s

sjoerdmeijer reviewed Jun 24, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Show resolved Hide resolved

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Outdated Show resolved Hide resolved

davemgreen reviewed Jun 24, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Outdated Show resolved Hide resolved

[llvm-exegesis] [AArch64] Reporting limitation in initializing operan…

f43b8e2

…d type in randomizeTargetMCOperand

lakshayk-nv requested review from sjoerdmeijer and abhilash1910 June 27, 2025 08:08

abhilash1910 reviewed Jun 30, 2025

View reviewed changes

lakshayk-nv added 2 commits August 13, 2025 23:04

[AArch64] Update randomizeTargetMCOperand to handle MSL immediate ope…

5a56401

…rands

[AArch64] Clang Format

cd435fb

sjoerdmeijer removed request for aaupov, paschalis-mpeis, QuietMisdreavus, xlauko, bcardosolopes, maksfb, rafaelauler, lanza, Endilll, daniel-grumberg, yota9, cyndyishida, ayermolo, DeinAlptraum, andykaylor and yozhu August 14, 2025 07:33

lakshayk-nv added 2 commits August 14, 2025 01:31

[llvm-exegesis] [AArch64] Updated tescase to check introduced OPERAND…

9ae23af

…_TYPES i.e. OPERAND_MSL_SHIFT_4S and OPERAND_MSL_SHIFT_2S

Merge branch 'main' into llvm-exegesis-uninit-operands

b0b4a5d

davemgreen reviewed Aug 14, 2025

View reviewed changes

llvm/lib/Target/AArch64/AArch64InstrFormats.td Outdated Show resolved Hide resolved

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Show resolved Hide resolved

llvm/lib/Target/AArch64/AArch64InstrFormats.td Outdated Show resolved Hide resolved

lakshayk-nv added 3 commits August 19, 2025 03:33

[AArch64] Introduce single OPERAND_MSL_SHIFT for both 2s and 4s opc…

555ddf7

…odes

[AArch64] Update MSL shift handling toopcode specific switch case

aa60946

[AArch64] Cleanup unrequired changes

6b593fe

boomanaiden154 reviewed Aug 19, 2025

View reviewed changes

llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp Outdated Show resolved Hide resolved

davemgreen reviewed Aug 20, 2025

View reviewed changes

lakshayk-nv added 4 commits August 20, 2025 02:29

[llvm-exegesis] [AArch64] Revert redundant PR_PAC_* constants

ab92f0f

[llvm-exegesis] [AArch64] Simplifies shift value for all move_vec_shi…

dbbed99

…ft to be msl llvm#8

[AArch64] Shifted defination OPERAND_MSL_SHIFT for readability NFC

bda8043

clang format

2d7fc2f

[llvm-exegesis] [AArch64] Resolving "not all operands are initialized by snippet generator" #142529

Are you sure you want to change the base?

[llvm-exegesis] [AArch64] Resolving "not all operands are initialized by snippet generator" #142529

Uh oh!

Conversation

lakshayk-nv commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 3, 2025

Uh oh!

github-actions bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boomanaiden154 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lakshayk-nv commented Jun 12, 2025

Uh oh!

sjoerdmeijer commented Jun 13, 2025

Uh oh!

boomanaiden154 commented Jun 16, 2025

Uh oh!

lakshayk-nv commented Jun 19, 2025

Uh oh!

sjoerdmeijer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhilash1910 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davemgreen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lakshayk-nv commented Jun 3, 2025 •

edited

Loading

github-actions bot commented Jun 3, 2025 •

edited

Loading

abhilash1910 left a comment •

edited

Loading