This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Ignore Implicit CPSR regs when lowering from Machine to MC operands
ClosedPublic

Authored by dmgreen on Aug 24 2019, 4:16 AM.

Details

Summary

The code here seems to date back to r134705, when tablegen lowering was first being added. I don't believe that we need to include CPSR implicit operands on the MCInst. This now works more like other backends (like AArch64), where all implicit registers are skipped.

This allows the AliasInst for CSEL's to match correctly, as can be seen in the test changes.

Diff Detail

Event Timeline

dmgreen created this revision.Aug 24 2019, 4:16 AM

This makes sense to me, and the verifier is happy! Does this affect how csels are disassembled when using the PC?

dmgreen updated this revision to Diff 218378.Sep 2 2019, 9:41 AM

I don't believe so, if I am understanding the question correctly.

  • The csels will use ZR in the encoding usually meant for PC, so can't use PC directly.
  • The asm/disass don't go through this codepath, which is just for lowering machine->mc (i.e only via codegen)
  • So all the existing disassembly/assembly shouldn't be effected by this as far as I understand (none of the tests are complaining)
  • And none of the codegen tests are failing either, so this doesn't seem to effect the asm output for anything that isn't csel.

We didn't seem to have tests for csel + pc though, so I've added some in the same place we were testing sp.

samparker accepted this revision.Sep 3 2019, 12:11 AM

Ok. LGTM.

This revision is now accepted and ready to land.Sep 3 2019, 12:11 AM
This revision was automatically updated to reflect the committed changes.