This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel] Select CSINC and CSINV for G_SELECT with constants
ClosedPublic

Authored by paquette on Nov 3 2020, 11:23 AM.

Details

Summary

Select the following:

  • G_SELECT cc, 0, 1 -> CSINC zreg, zreg, cc
  • G_SELECT cc 0, -1 -> CSINV zreg, zreg cc
  • G_SELECT cc, 1, f -> CSINC f, zreg, inv_cc
  • G_SELECT cc, -1, f -> CSINV f, zreg, inv_cc
  • G_SELECT cc, t, 1 -> CSINC t, zreg, cc
  • G_SELECT cc, t, -1 -> CSINC t, zreg, cc

(IR example: https://godbolt.org/z/YfPna9)

These correspond to a bunch of the AArch64csel patterns in AArch64InstrInfo.td.

Unfortunately, it doesn't seem like we can import patterns that use NZCV like those ones do. E.g.

def : Pat<(AArch64csel GPR32:$tval, (i32 1), (i32 imm:$cc), NZCV),
        (CSINCWr GPR32:$tval, WZR, (i32 imm:$cc))>;

So we have to manually select these for now.

This replaces selectSelectOpc with an emitSelect function, which performs these optimizations.

Diff Detail

Event Timeline

paquette created this revision.Nov 3 2020, 11:23 AM
paquette requested review of this revision.Nov 3 2020, 11:23 AM
paquette updated this revision to Diff 302686.Nov 3 2020, 2:22 PM
  • Forgot to use Is32Bit for all of the cases in the original diff.
  • Make the helper lambda return a bool for a follow-up commit.
aemerson added inline comments.Nov 3 2020, 7:57 PM
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
1003

Assert here that False RB is also FPR?

1082

Why do we need this as a lambda?

aemerson added inline comments.Nov 7 2020, 10:25 PM
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
1082

Ok I get why you wanted the lambda. Overall though given that you intend to introduce more lambdas, can you explicitly capture the variables, or pass them in as arguments?

paquette updated this revision to Diff 304586.Nov 11 2020, 10:53 AM
  • Explicitly capture variables in the lambda
  • Add an assert that checks that the true/false values for the G_SELECT have the same register bank
  • Rename the CSel variable to SelectInst, since it's not always going to be a CSel.
aemerson accepted this revision.Nov 12 2020, 9:48 AM

LGTM with nit.

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
1018

Don't need to capture Is32Bit by ref.

This revision is now accepted and ready to land.Nov 12 2020, 9:48 AM