Skip to content

Conversation

lakshayk-nv
Copy link
Contributor

@lakshayk-nv lakshayk-nv commented Sep 2, 2025

Reland #142529 (Resolving "not all operands are initialized by snippet generator")

Introduced changes in implementation of randomizeTargetMCOperand() for AArch64 that omitting OPERAND_SHIFT_MSL, OPERAND_PCREL to an immediate value of 264 and 8 respectively.
PS: Omitting MCOI::OPERAND_FIRST_TARGET/llvm:AArch64:OPERAND_IMPLICIT_IMM_0 similarly, to value 0. It was low hanging change thus added in this PR only.

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

[Reland Updates]

Updated tools/llvm-exegesis/AArch64/error-resolution.s which caused problem.
Test case was failing when there is uninitialised operands error coming from secondary/consumer instruction used by exegesis in latency mode required to chain up the assembly to ensure serial execution.

i.e. We get error message like UMOVvi16_idx0: Not all operands were initialized by the snippet generator for <<<any opcode other than UMOVvi16_idx0>>> opcode. but test case want to check like
# UMOVvi16_idx0_latency: ---. Thus the testcase fails.

/llvm-project/llvm/test/tools/llvm-exegesis/AArch64/error-resolution.s:65:26: error: UMOVvi16_idx0_latency: expected string not found in input
# UMOVvi16_idx0_latency: ---
                         ^
<stdin>:1:1: note: scanning from here
UMOVvi16_idx0: Not all operands were initialized by the snippet generator for LD1W_D_IMM opcode.
^

Input file: <stdin>
Check file: /llvm-project/llvm/test/tools/llvm-exegesis/AArch64/error-resolution.s

-dump-input=help explains the following input dump.

Input was:
<<<<<<
          1: UMOVvi16_idx0: Not all operands were initialized by the snippet generator for LD1W_D_IMM opcode. 
check:65     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
>>>>>>

--

********************
********************
Failed Tests (1):
  LLVM :: tools/llvm-exegesis/AArch64/error-resolution.s

[Why it fails (only sometimes)]

Exegesis in latency mode require the generated assembly to be chained to ensure serial execution,
For this exegesis add an additional consumer instruction for some instruction, which is chosen via a random seed.
Thus, it randomly fails whenever there is secondary consumer instruction (which is unsupported/throws error) added in generated assembly.

Please review: @sjoerdmeijer @davemgreen @boomanaiden154 @abhilash1910
Thanks!

… by the snippet generator" by omit OPERAND_UNKNOWN to Immediate
…nippet generation, omiting immediate valued 0.
… MSL immediate operands for 32-bit SIMD constants
…_TYPES i.e. OPERAND_MSL_SHIFT_4S and OPERAND_MSL_SHIFT_2S
@lakshayk-nv lakshayk-nv changed the title [llvm-exegesis] [AArch64] Reland #142529 Resolving "not all operands are initialized by snippet generator" [llvm-exegesis] [AArch64] Reland Resolving "not all operands are initialized by snippet generator" Sep 2, 2025
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

LGTM, assuming you've rerun the test quite a few times locally to ensure there aren't any flakes.


// Hacky temporary fix works by defaulting all OPERAND_UNKNOWN to
// immediate value 0, but this introduce illegal instruction error for below
// system instructions will need to be omitted with OperandType or opcode
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
but this introduces illegal instruction error for the below instructions.
System instructions ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comment. Thanks!

// - Type 8: imm at [23:16], [55:48], shift = 272 (0x110) → msl #16
// Corresponds AArch64_AM::encodeAdvSIMDModImmType7()
// But, v2s_msl and v4s_msl instructions accept either form,
// Thus, Arbitrarily chosing 264 (msl #8) for simplicity.
Copy link
Contributor

@abhilash1910 abhilash1910 Sep 2, 2025

Choose a reason for hiding this comment

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

Should we also count for 272 ? We may not want to enforce 264 always?
Edit: This can be extended for other imm operands which are defaulted to some value, probably in a separate patch- essentially "taking value as an arg" . @boomanaiden154 @sjoerdmeijer @davemgreen what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree, this area needs some work in follow ups, but let's start somewhere with this, so is okay as a start.

Copy link
Contributor

@abhilash1910 abhilash1910 left a comment

Choose a reason for hiding this comment

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

LGTM otherwise!

@sjoerdmeijer sjoerdmeijer merged commit ee71af4 into llvm:main Sep 3, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants