[AArch64][GlobalISel] Select scalar G_FMAXIMUM + G_FMINIMUM

[paquette]

Summary

Import the patterns via tablegen.

Putting this in AArch64InstrGISel.td assuming it wasn't in SelectionDAGCompat.td for a good reason.

[jroelofs]

llvm/test/CodeGen/AArch64/GlobalISel/select-fmaximum.mir

44

Am I reading the ARM/GISel docs correctly that G_FMAXIMUM corresponds to the FMAX instructions, G_FMAXNUM_IEEE corresponds to FMAXNM, and that there isn't direct support on ARMv8 for G_FMAXNUM?

A quick look at the SelectionDAG code suggests the opposite re: FMAXNM vs FMAXNM_IEEE though... so there's either a bug in my understanding, or SDAG got it wrong?

[paquette]

llvm/test/CodeGen/AArch64/GlobalISel/select-fmaximum.mir

44

Apparently AArch64InstrInfo.td says...

defm FMAXNM : TwoOperandFPData<0b0110, "fmaxnm", fmaxnum>;
defm FMAX   : TwoOperandFPData<0b0100, "fmax", fmaximum>;

So I'd assume

G_FMAXNUM -> fmaxnm
G_FMAXIMUM -> fmax

Maybe I'm wrong though?

[fhahn]

reverse ping :)

[aemerson]

Second reverse ping.

[paquette]

@aemerson wdyt about Jon's comment on the test?

[aemerson]

In D115381#3660965, @paquette wrote:

@aemerson wdyt about Jon's comment on the test?

I haven't a clue what's going on with signaling NaNs and llvm. I think the safest thing is to emulate existing codegen behavior and maybe someone else, @eli.friedman? might be able to explain.

[efriedma]

As far as I can tell, the AArch64 fmaxnm actually corresponds to ISD::FMAXNUM_IEEE. I wouldn't be surprised if SelectionDAG is wrong; rG687ec75d was implemented well after the AArch64 backend was written. Granted, SNaNs are rarely used, especially with floating-point exceptions disabled, so maybe nobody noticed.

[aemerson]

+ @arsenm

[arsenm]

Generic part LGTM, I have no idea about the AArch64 instruction behavior

llvm/lib/Target/AArch64/AArch64InstrGISel.td

244–245 ↗

(On Diff #392884)

This is obvious, LGTM

[arsenm]

llvm/lib/Target/AArch64/AArch64InstrGISel.td

244–245 ↗

(On Diff #392884)

Actually this doesn't LGTM. This belongs in generic code

[aemerson]

Since the correctness of FMAXIMUM/FMINIMUM aren't in question, this LGTM assuming we move it to SelectionDAGCompat.td instead.

[paquette]

Import the pattern everywhere + rebase test before pushing.

llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td

 def : GINodeEquiv<G_SMAX, smax>;
 def : GINodeEquiv<G_UMIN, umin>;
 def : GINodeEquiv<G_UMAX, umax>;
 def : GINodeEquiv<G_ABS, abs>;
 def : GINodeEquiv<G_FMINNUM, fminnum>;
 def : GINodeEquiv<G_FMAXNUM, fmaxnum>;
 def : GINodeEquiv<G_FMINNUM_IEEE, fminnum_ieee>;
 def : GINodeEquiv<G_FMAXNUM_IEEE, fmaxnum_ieee>;
+def : GINodeEquiv<G_FMAXIMUM, fmaximum>;
+def : GINodeEquiv<G_FMINIMUM, fminimum>;
 def : GINodeEquiv<G_READCYCLECOUNTER, readcyclecounter>;
 def : GINodeEquiv<G_ROTR, rotr>;
 def : GINodeEquiv<G_ROTL, rotl>;
 def : GINodeEquiv<G_LROUND, lround>;
 def : GINodeEquiv<G_LLROUND, llround>;
 
 def : GINodeEquiv<G_STRICT_FADD, strict_fadd>;
 def : GINodeEquiv<G_STRICT_FSUB, strict_fsub>;

llvm/test/CodeGen/AArch64/GlobalISel/select-fmaximum.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=aarch64 -mattr=+fullfp16 -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
+
+...
+---
+name:            s16
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $h0, $h1
+
+    ; CHECK-LABEL: name: s16
+    ; CHECK: liveins: $h0, $h1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:fpr16 = COPY $h0
+    ; CHECK-NEXT: %b:fpr16 = COPY $h1
+    ; CHECK-NEXT: %select_me:fpr16 = nofpexcept FMAXHrr %a, %b
+    ; CHECK-NEXT: $h0 = COPY %select_me
+    ; CHECK-NEXT: RET_ReallyLR implicit $h0
+    %a:fpr(s16) = COPY $h0
+    %b:fpr(s16) = COPY $h1
+    %select_me:fpr(s16) = G_FMAXIMUM %a, %b
+    $h0 = COPY %select_me(s16)
+    RET_ReallyLR implicit $h0
+
+...
+---
+name:            s32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $s0, $s1
+
+    ; CHECK-LABEL: name: s32
+    ; CHECK: liveins: $s0, $s1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:fpr32 = COPY $s0
+    ; CHECK-NEXT: %b:fpr32 = COPY $s1
+    ; CHECK-NEXT: %select_me:fpr32 = nofpexcept FMAXSrr %a, %b
+    ; CHECK-NEXT: $s0 = COPY %select_me
+    ; CHECK-NEXT: RET_ReallyLR implicit $s0
+    %a:fpr(s32) = COPY $s0
+    %b:fpr(s32) = COPY $s1
+    %select_me:fpr(s32) = G_FMAXIMUM %a, %b
+    $s0 = COPY %select_me(s32)
+    RET_ReallyLR implicit $s0
+
+...
+---
+name:            s64
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $d0, $d1
+
+    ; CHECK-LABEL: name: s64
+    ; CHECK: liveins: $d0, $d1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:fpr64 = COPY $d0
+    ; CHECK-NEXT: %b:fpr64 = COPY $d1
+    ; CHECK-NEXT: %select_me:fpr64 = nofpexcept FMAXDrr %a, %b
+    ; CHECK-NEXT: $d0 = COPY %select_me
+    ; CHECK-NEXT: RET_ReallyLR implicit $d0
+    %a:fpr(s64) = COPY $d0
+    %b:fpr(s64) = COPY $d1
+    %select_me:fpr(s64) = G_FMAXIMUM %a, %b
+    $d0 = COPY %select_me(s64)
+    RET_ReallyLR implicit $d0

llvm/test/CodeGen/AArch64/GlobalISel/select-fminimum.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=aarch64 -mattr=+fullfp16 -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
+
+...
+---
+name:            s16
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $h0, $h1
+
+    ; CHECK-LABEL: name: s16
+    ; CHECK: liveins: $h0, $h1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:fpr16 = COPY $h0
+    ; CHECK-NEXT: %b:fpr16 = COPY $h1
+    ; CHECK-NEXT: %select_me:fpr16 = nofpexcept FMINHrr %a, %b
+    ; CHECK-NEXT: $h0 = COPY %select_me
+    ; CHECK-NEXT: RET_ReallyLR implicit $h0
+    %a:fpr(s16) = COPY $h0
+    %b:fpr(s16) = COPY $h1
+    %select_me:fpr(s16) = G_FMINIMUM %a, %b
+    $h0 = COPY %select_me(s16)
+    RET_ReallyLR implicit $h0
+
+...
+---
+name:            s32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $s0, $s1
+
+    ; CHECK-LABEL: name: s32
+    ; CHECK: liveins: $s0, $s1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:fpr32 = COPY $s0
+    ; CHECK-NEXT: %b:fpr32 = COPY $s1
+    ; CHECK-NEXT: %select_me:fpr32 = nofpexcept FMINSrr %a, %b
+    ; CHECK-NEXT: $s0 = COPY %select_me
+    ; CHECK-NEXT: RET_ReallyLR implicit $s0
+    %a:fpr(s32) = COPY $s0
+    %b:fpr(s32) = COPY $s1
+    %select_me:fpr(s32) = G_FMINIMUM %a, %b
+    $s0 = COPY %select_me(s32)
+    RET_ReallyLR implicit $s0
+
+...
+---
+name:            s64
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $d0, $d1
+
+    ; CHECK-LABEL: name: s64
+    ; CHECK: liveins: $d0, $d1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:fpr64 = COPY $d0
+    ; CHECK-NEXT: %b:fpr64 = COPY $d1
+    ; CHECK-NEXT: %select_me:fpr64 = nofpexcept FMINDrr %a, %b
+    ; CHECK-NEXT: $d0 = COPY %select_me
+    ; CHECK-NEXT: RET_ReallyLR implicit $d0
+    %a:fpr(s64) = COPY $d0
+    %b:fpr(s64) = COPY $d1
+    %select_me:fpr(s64) = G_FMINIMUM %a, %b
+    $d0 = COPY %select_me(s64)
+    RET_ReallyLR implicit $d0