PowerPC: add mcrxr instruction support - #3065
Open
StilesCrisis wants to merge 1 commit into
Open
Conversation
mcrxr (Move to Condition Register from XER) was part of the original
POWER/PowerPC architecture, but was dropped from Book I as of PowerPC
ISA 2.03 - superseded by the non-destructive mcrxrx on ISA 3.0+
implementations, which Capstone already decodes. It moves the top 4
XER bits (SO, OV, CA and a reserved bit) into a chosen CR field and
clears them in XER. It's still emitted by/found in legacy PowerPC and
POWER binaries, but neither v5 nor v6 currently decode it.
Encoding is X-form, opcode 31, XO 512 (same XForm_17-derived layout
LLVM already uses for mcrxrx's XO 576, just one operand: BF). That
slot is unused by any other PPC instruction, so this is added
unconditionally, without a subtarget predicate.
Since v6's PPC module is generated from capstone-engine/llvm-capstone
via Auto-Sync (see docs/ARCHITECTURE.md, suite/auto-sync/intro.md),
the actual instruction definition (a `def MCRXR : X_BF3<31, 512, ...>`
next to the existing mcrxrx, plus a P9/P10 scheduling-resource entry
each, mirroring mcrxrx) lives in that fork's PPCInstrInfo.td /
P9InstrResources.td / P10InstrResources.td, not in this repository.
This commit is the regenerated output of running `ASUpdater -a PPC`
against that change.
To keep the regen isolated to mcrxr, llvm-capstone's auto-sync branch
was pinned to commit f6a22fb89 ("Tread crbitm as register",
2024-12-06) rather than its current tip: regenerating PPC's tables
from current HEAD at the outset pulled in ~14 months of unrelated
upstream drift (register/instruction renumbering, alias/format
reclassification) alongside mcrxr, some of which broke existing
tests. f6a22fb89 was confirmed - by regenerating without the mcrxr
change and diffing byte-for-byte against every currently checked-in
PPC*.inc file - to be the exact commit the current tables were last
generated from, so this diff is now purely the mcrxr addition (plus
the expected cascading enum renumbering that coincidentally comes
from inserting any single instruction into a generated, sequentially
numbered table - see e.g. b25aa84/93a104c0/eed87476 for prior
instances of this same class of change).
include/capstone/ppc.h's generated enum region was hand-patched
(single `PPC_INS_MCRXR,` line) rather than via ASUpdater's automated
PatchArchHeader step: that step's underlying tblgen-generated
PPCGenCS*Enum.inc intermediates were reproducibly corrupted (stray
embedded NUL bytes splitting identifiers, e.g. PPC_INS_ALIAS_MR_ ->
PPC_INS_ALIAS_<NUL>R_) when built with this MSVC/Windows toolchain -
apparently pre-existing and unrelated to mcrxr, since it reproduced
identically across reruns and touched aliases mcrxr has none of.
Worth a separate look on Windows; sidestepped here since the needed
header change is a single, easily hand-verified line.
No hand-written files (PPCDisassembler.c, PPCInstPrinter.c,
PPCMapping.c) needed changes - mcrxr reuses the crrc-typed BF operand
mcrxrx already exercises.
Added an entry to tests/details/ppc.yaml (cr0 and cr1 forms) verified
against real `cs_detail` output. Confirmed no regressions: the full
suite/auto-sync/vendor cstest run against tests/details/ppc.yaml
(18/18) and tests/MC/PowerPC (5090/5090, 920 skipped) both pass,
before and after this change.
A companion PR to capstone-engine/llvm-capstone carrying the actual
.td definitions this was regenerated from should follow, so the next
official Auto-Sync run doesn't drop mcrxr again.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012jWces2QBvBWg9m3NfxH4v
Collaborator
|
Thanks, will check it tomorrow. But please follow the PR template. And be aware that we will very soon require people to hand-write their PR descriptions: #3064 |
Author
|
Alright, thanks, updated with the template |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Your checklist for this pull request
Detailed description
mcrxr. This opcode was eventually dropped from the Power standard, but was emitted by CW 1.2.5n.mcrxrx(XO 576), singleBF/crf operand. XO 512 is otherwise unused — noRequires<>/feature gate needed..tddef update exists in the llvm-capstone repo here: PowerPC: add mcrxr instruction definition llvm-capstone#94.mcrxr, llvm-capstone'sauto-syncbranch was pinned to commitf6a22fb89(2024-12-06).PPCDisassembler.c,PPCInstPrinter.c,PPCMapping.c) needed changes.Test plan
cr0/cr1entries totests/details/ppc.yaml.suite/cstestagainsttests/details/ppc.yaml: passedsuite/cstestagainsttests/MC/PowerPC: passedcstool -dcheck:7c800400→mcrxr cr1.Closing issues
closes #3066