[PATCH] D139925: [AArch64] Add new v9.4-A PM pstate system register

[Sam Elliott via Phabricator via llvm-commits]

lenary added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1793
+  let DecoderMethod = "DecodeSystemPStateImm0_1Instruction";
   // MSRpstateI aliases with MSRI. When the MSRpstateI decoder method returns
   // Fail the decoder should attempt to decode the instruction as MSRI.
----------------
pratlucas wrote:
> pratlucas wrote:
> > lenary wrote:
> > > Do we have decoding tests for the case where you fallback from `MSRpstateImm0_15` to `MSRpstateImm0_1` to `MSRI`? This would presumably be a test where we use an unallocated MSR (imm) encoding?
> > The fallback applies every time the decoder method fails due to the lack of the required feature for a particular pstate register.
> > This is currently covered by the negative tests for `PAN`, `UAO`, `DIT`, `SSBS`, and `TCO`.
> On a deeper look, this existing behaviour doesn't look like the best approach to handle those situations. `MSRI` ends up decoding the instruction using the register form of the `msr` instruction, generating a somewhat meaningless result.
> I'll have a look into an alternative for that, but given that's existing behaviour the change may go into a separate patch.
> I'll have a look into an alternative for that, but given that's existing behaviour the change may go into a separate patch.

As long as we can parse whatever we produce, I think it should be ok. Maybe worth checking what binutils prints in these cases?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139925/new/

https://reviews.llvm.org/D139925