This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Refactor AArch64InstrInfo::isAsCheapAsAMove (NFC)
ClosedPublic

Authored by chill on Jul 7 2023, 8:15 AM.

Details

Summary
  • remove FeatureCustomCheapAsMoveHandling: when you have target features affecting isAsCheapAsAMove that can be given on command line or passed via attributes, then every sub-target effectively has custom handling
  • remove special handling of FMOVD0/etc: FVMOV with an immediate zero operand is never[1] more expensive tha an FMOV with a register operand.
  • remove special handling of COPY - copy is trivially as cheap as itself
  • make the function default to the MachineInstr attribute isAsCheapAsAMove
  • remove special handling of ANDWrr/etc and of ANDWri/etc: the fallback MachineInstr attribute is already non-zero.
  • remove special handling of ADDWri/SUBWri/ADDXri/SUBXri - there are always[1] one cycle latency with maximum (for the micro-architecture) throughput
  • check if MOVi32Imm/MOVi64Imm can be expanded into a "cheap" sequence of instructions

    There is a little twist with determining whether a MOVi32Imm`/MOVi64Imm is "as-cheap-as-a-move". Even if one of these pseudo-instructions needs to be expanded to more than one MOVZ, MOVN, or MOVK instructions, materialisation may be preferrable to allocating a register to hold the constant. For the moment a cutoff at two instructions seems like a reasonable compromise.

    [1] according to 19 software optimisation manuals

Diff Detail

Event Timeline

chill created this revision.Jul 7 2023, 8:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2023, 8:15 AM
chill requested review of this revision.Jul 7 2023, 8:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2023, 8:15 AM
chill updated this revision to Diff 538158.Jul 7 2023, 8:40 AM
chill added reviewers: efriedma, t.p.northover, dmgreen.
chill updated this revision to Diff 542957.Jul 21 2023, 8:57 AM
chill edited the summary of this revision. (Show Details)
chill edited the summary of this revision. (Show Details)
dmgreen added inline comments.Jul 24 2023, 2:37 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
898

What do you mean by micro-architecture dependent?

900

Is it worth using AArch64_IMM::expandMOVImm with checking the Insns.size() <= 2? It might be a little slower, but more precise and should handle any canBeExpandedToORR / canBeExpandedToMOVZNK /anything else it learns about in the future.

Does this need to be LLVM_ATTRIBUTE_ALWAYS_INLINE? Those kinds of decisions are usually best left to the optimizer.

chill updated this revision to Diff 543542.Jul 24 2023, 7:30 AM
chill edited the summary of this revision. (Show Details)
chill marked 2 inline comments as done.
chill added inline comments.
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
898

Existing comment, perhaps means use a sub-target hook, instead of a target hook.

dmgreen accepted this revision.Jul 27 2023, 12:52 AM

Thanks. LGTM.

This revision is now accepted and ready to land.Jul 27 2023, 12:52 AM
chill updated this revision to Diff 557176.Sep 21 2023, 6:58 AM
chill marked an inline comment as done.

Update for some scheduling differences in the test

This revision was automatically updated to reflect the committed changes.