Page MenuHomePhabricator

[AArch64] Mark all instructions that read/write FPCR as doing so
Changes PlannedPublic

Authored by john.brawn on Mar 17 2022, 10:26 AM.

Details

Summary

All instructions that can raise fp exceptions also read FPCR, with the only other instructions that interact with it being the MSR/MRS to write/read FPCR.

Introducing an FPCR register also requires fixing an assumption in invalidateWindowsRegisterPairing in AArch64FrameLowering.cpp that the enum value of AArch64::LR is one greater than that of AArch64::FP,
which it no longer is due to the registers being arranged in alphabetical order.

This change unfortunately means a large number of mir tests need to be adjusted due to instructions now requiring an implicit fpcr operand to be present.

Diff Detail

Event Timeline

john.brawn created this revision.Mar 17 2022, 10:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 10:26 AM
john.brawn requested review of this revision.Mar 17 2022, 10:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 10:26 AM
simon_tatham added inline comments.Mar 18 2022, 3:27 AM
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
2225

I don't know this code well, but just looking at this if statement in isolation, in the case where Reg1==FP, the previous version of the condition would have required Reg2==LR, whereas this one will accept either Reg2==LR or Reg2=FPCR.

Or is there some condition at the call sites which means nothing but GPRs will ever be passed to this function in the first place, so that the (FP,FPCR) pair will never trigger that false positive?

john.brawn planned changes to this revision.Apr 6 2022, 7:44 AM

This is interacting badly with MachineCSE, as the presence of a physical register prevents it from optimising in some cases. I think the fix is in MachineCSE, but removing this patch from review while I look at it.