Skip to content

synctools: generalize asmwriter.py's AArch64-only alias/mnemonic logic - #3053

Open
StilesCrisis wants to merge 1 commit into
capstone-engine:v5from
StilesCrisis:fix-asmwriter-arch-generalization
Open

synctools: generalize asmwriter.py's AArch64-only alias/mnemonic logic#3053
StilesCrisis wants to merge 1 commit into
capstone-engine:v5from
StilesCrisis:fix-asmwriter-arch-generalization

Conversation

@StilesCrisis

Copy link
Copy Markdown

Problem

suite/synctools/asmwriter.py (the script that converts raw llvm-tblgen -gen-asm-writer output into capstone's C style) has AArch64-only logic hardcoded into several places, dating to when the script was written for AArch64 in 2022 and never exercised against another target since. Running it against a non-AArch64 target's raw tblgen output produced by a reasonably modern LLVM breaks in several ways - found while regenerating PowerPC's asm-writer output for #3052.

Bugs fixed

  • getRegisterName/getMnemonic's "const char *AArch64InstPrinter::" skip rule was a literal string match, so for any other target it also matched (and silently swallowed) that target's own real getRegisterName definition line - e.g. PPC's "const char *PPCInstPrinter::getRegisterName(...)" matches the same substring pattern. Result: a getRegisterName body with no function signature at all. Now excludes getRegisterName explicitly, and matches the target's own class name instead of a hardcoded AArch64InstPrinter.
  • getMnemonic had no translation for newer LLVM's early-out guard (if (Bits == 0) return {nullptr, Bits};), which doesn't exist in the older LLVM output this script was originally written against.
  • Newer LLVM wraps register handles in MCRegister, so getRegisterName's raw definition now includes a RegNo = Reg.id(); redeclaration line that conflicts with this script's hardcoded plain-unsigned-parameter signature.
  • The injected printAliasInstr alias-matching block hardcoded AArch64InstPrinterValidateMCOperand and AArch64's SME tile-slice-indexing bracket syntax (za[w8, 0]) unconditionally. Validator name is now parameterized by target; SME bracket handling now only applies for AArch64/ARM64, with a plain set_mem_access()-only fallback for every other target (none of which have SME syntax).

Each fix was confirmed necessary by an actual compiler/parse error while iterating toward a from-scratch PPC rebuild, not speculative.

Scope note

This is a synctools script fix, not a PPC-specific change - it generalizes logic that only worked for AArch64 to work for any target regenerated against a comparably modern LLVM. It's independent of #3052, which uses an older (LLVM 7.0.1-vintage) toolchain that doesn't hit any of these code paths at all.

One related note for anyone regenerating a target against modern LLVM specifically: if that target has zero K_Custom AliasPatternConds (true for PPC, as of writing), the injected <Target>InstPrinterValidateMCOperand call in the alias-matching block is unreachable but still needs a stub of that name in the target's InstPrinter.c to link - this script doesn't generate that stub itself, since it's C source, not template output.

While attempting to regenerate PPC's AsmWriter output end-to-end (see
previous commit), hit several real bugs in this script - all in logic
that was hand-written specifically for AArch64 in 2022 and never
exercised against any other target since:

- getMnemonic/getRegisterName's "const char *AArch64InstPrinter::"
  skip rule was hardcoded to the literal string "AArch64InstPrinter::",
  so it silently swallowed PPC's real getRegisterName definition line
  too (both match "const char *PPCInstPrinter::getRegisterName(...)"),
  producing a getRegisterName body with no function signature at all.
  Now excludes getRegisterName explicitly.

- getMnemonic's std::pair-returning raw signature had no matching
  early-out translation for the "if (Bits == 0) return {nullptr,
  Bits};" guard that newer LLVM adds - fixed to fold into the same
  uint64_t-returning convention already used for the main return path.

- Newer LLVM wraps register handles in MCRegister, so getRegisterName's
  raw definition now reads "getRegisterName(MCRegister Reg) { unsigned
  RegNo = Reg.id(); ... }" - this leftover redeclaration line conflicted
  with our hardcoded plain-unsigned-parameter signature. Now stripped.

- The injected alias-matching block's AArch64InstPrinterValidateMCOperand
  call and its isSME/set_sme_index handling (AArch64 SME tile-slice
  indexing syntax, e.g. "za[w8, 0]") were both hardcoded literals.
  Parameterized the validator name by target; SME handling now only
  applies for AArch64/ARM64, with a plain always-set_mem_access fallback
  for every other target (which never has SME bracket syntax).

Each fix was confirmed necessary by direct compiler error (not
speculative) while iterating toward a from-scratch PPC rebuild. See the
previous commit for what remains unverified about that full rebuild.
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.

1 participant