This is an archive of the discontinued LLVM Phabricator instance.

[GISel]: Add combine for G_FABS to G_FABS
ClosedPublic

Authored by mkitzan on Sep 11 2020, 7:10 PM.

Details

Summary

Patch adds one new GICombinerRule for G_FABS. The combine rule folds G_FABS(G_FABS(X)) to G_FABS(X). Patch additionally adds new combiner tests for the AArch64 target to test this new combiner rule.

Diff Detail

Event Timeline

mkitzan created this revision.Sep 11 2020, 7:10 PM
mkitzan requested review of this revision.Sep 11 2020, 7:10 PM
arsenm added inline comments.Sep 12 2020, 7:51 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
2028–2031

replaceRegWith?

mkitzan updated this revision to Diff 291603.Sep 14 2020, 10:06 AM
  • Used replaceRegWith instead of building a copy
paquette added inline comments.Sep 14 2020, 10:21 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1826

Maybe simpler?

return mi_match(MI.getOperand(1).getReg(), MRI, m_GFabs(m_reg(Src)));
paquette added inline comments.Sep 14 2020, 10:24 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1826

oh wait no that won't work, that'll give the source of the other G_FABS nevermind

arsenm accepted this revision.Sep 14 2020, 1:19 PM

LGTM. Maybe should factor this into a combine_idempotent that checks the inner opcode matches for other operations later

This revision is now accepted and ready to land.Sep 14 2020, 1:19 PM

Thanks for the review. Still don't have commit access, will need someone to commit on my behalf. Thanks in advance!

I can commit on your behalf.
Also we'll need to figure out how to get you access.

Thanks Aditya!