[all-commits] [llvm/llvm-project] 1076b3: [GlobalISel] Combine select + fcmp to fminnum/fmax...

Jessica Paquette via All-commits all-commits at lists.llvm.org
Fri Sep 16 13:36:15 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1076b31da8da8854e1dfca8f1a9a03de9fd4f8f5
      https://github.com/llvm/llvm-project/commit/1076b31da8da8854e1dfca8f1a9a03de9fd4f8f5
  Author: Jessica Paquette <jpaquette at apple.com>
  Date:   2022-09-16 (Fri, 16 Sep 2022)

  Changed paths:
    M llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
    M llvm/include/llvm/Target/GlobalISel/Combine.td
    M llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
    M llvm/lib/Target/AArch64/AArch64Combine.td

  Log Message:
  -----------
  [GlobalISel] Combine select + fcmp to fminnum/fmaxnum/fminimum/fmaximum

This is a partial port of the code used by the SelectionDAGBuilder to
translate selects.

In particular, see matchSelectPattern in ValueTracking.cpp. This is a
GISel-equivalent of the portion which handles fminnum/fmaxnum/fminimum/fmaximum.

I tried to set it up so it'd be easy to add the non-FP cases. Those are simpler.
On the AArch64-end, it seems like the FP cases are more important for perf
right now, so I bit the bullet and went at the more complicated problem. :)

I elected to do this as a post-legalize combine rather than in the
IRTranslator because

Deciding which fmax/fmin to use can depend on legalization rules
Philosophically-speaking (TM), putting it in a combine just feels cleaner

Being able to enable/disable the combine is handy
Another option would be to use the ValueTracking code in the IRTranslator and
match what SelectionDAGBuilder::visitSelect does. I think that may be somewhat
annoying since we'd need to write lowerings back into the selects in the
legalizer. I'm not strongly opposed to the approach.

We'd also want to be careful with vector selects once that's implemented,
which explicitly check if a vector select is legal on the target. That'd
probably need a hook.

>From what I can tell, doing this as a combine is probably a cleaner option
long-term.

Differential Revision: https://reviews.llvm.org/D116702




More information about the All-commits mailing list