This is an archive of the discontinued LLVM Phabricator instance.

[GISel] Add identity and fneg combines for G_FMUL
Needs RevisionPublic

Authored by mkitzan on May 19 2022, 12:15 PM.

Details

Summary

Patch adds two simple combines:

  • G_FMUL(x, 1.0) -> x
  • G_FMUL(G_FNEG(x), G_FNEG(y)) -> G_FMUL(x, y)

Patch additionally adds new combine tests for AArch64 target for these new rules.
(Split off from https://reviews.llvm.org/D87870)

Diff Detail

Event Timeline

mkitzan created this revision.May 19 2022, 12:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2022, 12:15 PM
mkitzan requested review of this revision.May 19 2022, 12:15 PM
foad added a comment.May 20 2022, 3:56 AM

General question about all these combines: why do we need them? Why wouldn't they be done at the IR level, or are they cleaning up code that could reasonably be introduced by legalization?

These are combines which seem generally beneficial (not architecture specific) from the closed source backend I work on. They are beneficial for our backend, so we have been upstreaming these combines to improve GISel.

arsenm added inline comments.Jan 27 2023, 7:39 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
2434–2435

This is illegal, no reason to check for it. Really we should just consistently use Register in the API

arsenm requested changes to this revision.Aug 17 2023, 4:23 PM

Can you try using new tablegen combiner support?

This revision now requires changes to proceed.Aug 17 2023, 4:23 PM