DAG: Handle inversion of fcSubnormal | fcZero

[arsenm]

Summary

There are a number of more test combinations here that
can be done together and reduce the number of instructions.

[sepavloff]

llvm/include/llvm/CodeGen/CodeGenCommonISel.h

228–229

Doxygen comments must be on separate lines.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

7333 ↗

(On Diff #494320)

This variable seems unused. It is possible to remove it?

llvm/test/CodeGen/X86/is_fpclass.ll

889–896

This code looks incorrect. The argument is compared against 0.0 and the result is true if the comparison is unordered or not equal.

[arsenm]

Fix inverted case

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

7333 ↗

(On Diff #494320)

Yes but I was trying to document changes that need to be made to match the DAG version

llvm/test/CodeGen/X86/is_fpclass.ll

889–896

Looks like I broke the inverted case and ended up fixing it in one of my later patches

[arsenm]

ping

[sepavloff]

llvm/test/CodeGen/X86/is_fpclass.ll

889–896

Not sure if x87 has DAZ mode. Corresponding flag is in MXCSR only and it is not clear if MXCSR affects operations in x87. @jcranmer-intel, could you please clarify this point?

[arsenm]

llvm/test/CodeGen/X86/is_fpclass.ll

889–896

The attribute says codegen can assume this. If the hardware doesn't actually support the mode this is just undefined usage which doesn't matter

[sepavloff]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

8071

Why ISD::SETONE? If condition is "ordered and equal", then inversion of it is "unordered or not equal", isn't it?

[arsenm]

Undo opcode change

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

8071

I don't really remember what I was thinking here

[sepavloff]

LGTM.

llvm/lib/CodeGen/CodeGenCommonISel.cpp

179

[arsenm]

64df9573a7ab0ab4a0743978086a8bfd1380a238

llvm/include/llvm/CodeGen/CodeGenCommonISel.h

 /// into the physical registers right before the terminator creating a
 /// ``Terminator Sequence''. This function is searching for the beginning of the
 /// terminator sequence so that we can ensure that we splice off not just the
 /// terminator, but additionally the copies that move the vregs into the
 /// physical registers.
 MachineBasicBlock::iterator
 findSplitPointForStackProtector(MachineBasicBlock *BB,
                                 const TargetInstrInfo &TII);
-/// Evaluates if the specified FP class test is an inversion of a simpler test.
-/// An example is the test "inf|normal|subnormal|zero", which is an inversion
-/// of "nan".
+
+/// Evaluates if the specified FP class test is better performed as the inverse
+/// (i.e. fewer instructions should be required to lower it).  An example is the
+/// test "inf|normal|subnormal|zero", which is an inversion of "nan".
 /// \param Test The test as specified in 'is_fpclass' intrinsic invocation.
-/// \returns The inverted test, or zero, if inversion does not produce simpler
-/// test.
-FPClassTest getInvertedFPClassTest(FPClassTest Test);
+/// \returns The inverted test, or fcNone, if inversion does not produce a
+/// simpler test.
+FPClassTest invertFPClassTestIfSimpler(FPClassTest Test);
 
 /// Assuming the instruction \p MI is going to be deleted, attempt to salvage
 /// debug users of \p MI by writing the effect of \p MI in a DIExpression.
 void salvageDebugInfoForDbgValue(const MachineRegisterInfo &MRI,
                                  MachineInstr &MI,
                                  ArrayRef<MachineOperand *> DbgUsers);
 
 } // namespace llvm
 
 #endif // LLVM_CODEGEN_CODEGENCOMMONISEL_H

llvm/lib/CodeGen/CodeGenCommonISel.cpp

while (MIIsInTerminatorSequence(*Previous)) {
     if (Previous == Start)
       break;
     --Previous;
   }
 
   return SplitPoint;
 }
 
-FPClassTest llvm::getInvertedFPClassTest(FPClassTest Test) {
+FPClassTest llvm::invertFPClassTestIfSimpler(FPClassTest Test) {
   FPClassTest InvertedTest = static_cast<FPClassTest>(~Test & fcAllFlags);
-  switch (InvertedTest) {
-  default:
-    break;
+  // Pick the direction with fewer tests
+  // TODO: Handle more combinations of cases that can be handled together
+  switch (static_cast<unsigned>(InvertedTest)) {
   case fcNan:
   case fcSNan:
   case fcQNan:
   case fcInf:
   case fcPosInf:
   case fcNegInf:
   case fcNormal:
   case fcPosNormal:
   case fcNegNormal:
   case fcSubnormal:
   case fcPosSubnormal:
   case fcNegSubnormal:
   case fcZero:
   case fcPosZero:
   case fcNegZero:
   case fcFinite:
   case fcPosFinite:
   case fcNegFinite:
+  case fcSubnormal | fcZero:
     return InvertedTest;
-  }
+  default:
     return fcNone;
   }
 
+  llvm_unreachable("covered FPClassTest");
+}
+
 static MachineOperand *getSalvageOpsForCopy(const MachineRegisterInfo &MRI,
                                             MachineInstr &Copy) {
   assert(Copy.getOpcode() == TargetOpcode::COPY && "Must be a COPY");
 
   return &Copy.getOperand(1);
 }
 
 static MachineOperand *getSalvageOpsForTrunc(const MachineRegisterInfo &MRI,

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

SDValue TargetLowering::expandFMINNUM_FMAXNUM(SDNode *Node,
   return SDValue();
 }
 
 /// If this FPClassTest can be performed with a fcmp to 0, return the test mask
 /// for the floating-point mode.
 static FPClassTest isFCmpEqualZero(FPClassTest Test,
                                    const fltSemantics &Semantics,
                                    const MachineFunction &MF) {
+  // TODO: Handle unordered compares
   if (Test == fcZero &&
       MF.getDenormalMode(Semantics).Input == DenormalMode::IEEE)
     return fcZero;
   if (Test == (fcZero | fcSubnormal) &&
       MF.getDenormalMode(Semantics).inputsAreZero())
     return static_cast<FPClassTest>(fcZero | fcSubnormal);
   return fcNone;
 }
if (OperandVT == MVT::ppcf128) {
     Op = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::f64, Op,
                      DAG.getConstant(1, DL, MVT::i32));
     OperandVT = MVT::f64;
   }
 
   // Some checks may be represented as inversion of simpler check, for example
   // "inf|normal|subnormal|zero" => !"nan".
   bool IsInverted = false;
-  if (FPClassTest InvertedCheck = getInvertedFPClassTest(TestMask)) {
+
+  if (FPClassTest InvertedCheck = invertFPClassTestIfSimpler(TestMask)) {
     IsInverted = true;
     TestMask = InvertedCheck;
   }
 
   // Floating-point type properties.
   EVT ScalarFloatVT = OperandVT.getScalarType();
   const Type *FloatTy = ScalarFloatVT.getTypeForEVT(*DAG.getContext());
   const llvm::fltSemantics &Semantics = FloatTy->getFltSemantics();
   bool IsF80 = (ScalarFloatVT == MVT::f80);
 
   // Some checks can be implemented using float comparisons, if floating point
   // exceptions are ignored.
   if (Flags.hasNoFPExcept() &&
       // TODO: Should check isCondCodeLegal
       isOperationLegalOrCustom(ISD::SETCC, OperandVT.getScalarType())) {
     if (isFCmpEqualZero(TestMask, Semantics, DAG.getMachineFunction()) !=
         fcNone) {
       // If denormals could be implicitly treated as 0, this is not equivalent
       // to a compare with 0 since it will also be true for denormals.
       return DAG.getSetCC(DL, ResultVT, Op,
                           DAG.getConstantFP(0.0, DL, OperandVT),
-                          IsInverted ? ISD::SETUNE : ISD::SETOEQ);
+                          IsInverted ? ISD::SETONE : ISD::SETOEQ);
     }
 
     if (TestMask == fcNan)
       return DAG.getSetCC(DL, ResultVT, Op, Op,
                           IsInverted ? ISD::SETO : ISD::SETUO);
   }
 
   // FIXME: Placeholder until FPClassTest is marked as a BitmaskEnum.
else if ((Test & fcFinite) == fcFinite) {
     Test &= ~fcNegFinite;
   }
   appendResult(PartialRes);
 
   if (unsigned PartialCheck = Test & (fcZero | fcSubnormal)) {
     // fcZero | fcSubnormal => test all exponent bits are 0
     // TODO: Handle sign bit specific cases
     if (PartialCheck == (fcZero | fcSubnormal)) {
-      assert(!IsInverted && "should handle inverted case");
-
       SDValue ExpBits = DAG.getNode(ISD::AND, DL, IntVT, OpAsInt, ExpMaskV);
       SDValue ExpIsZero =
           DAG.getSetCC(DL, ResultVT, ExpBits, ZeroV, ISD::SETEQ);
       appendResult(ExpIsZero);
       Test &= ~PartialCheck & fcAllFlags;
     }
   }
 

llvm/test/CodeGen/AMDGPU/llvm.is.fpclass.f16.ll

entry:
   ret i1 %class
 }
 
 define i1 @not_issubnormal_or_zero_f16(half %x) {
 ; GFX7SELDAG-LABEL: not_issubnormal_or_zero_f16:
 ; GFX7SELDAG:       ; %bb.0: ; %entry
 ; GFX7SELDAG-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7SELDAG-NEXT:    v_cvt_f16_f32_e32 v0, v0
-; GFX7SELDAG-NEXT:    s_movk_i32 s4, 0x7bff
-; GFX7SELDAG-NEXT:    s_movk_i32 s6, 0x7800
-; GFX7SELDAG-NEXT:    v_and_b32_e32 v0, 0x7fff, v0
-; GFX7SELDAG-NEXT:    v_cmp_lt_i32_e64 s[4:5], s4, v0
-; GFX7SELDAG-NEXT:    v_add_i32_e32 v0, vcc, 0xfffffc00, v0
-; GFX7SELDAG-NEXT:    v_and_b32_e32 v0, 0xffff, v0
-; GFX7SELDAG-NEXT:    v_cmp_gt_u32_e32 vcc, s6, v0
-; GFX7SELDAG-NEXT:    s_or_b64 s[4:5], s[4:5], vcc
-; GFX7SELDAG-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s[4:5]
+; GFX7SELDAG-NEXT:    v_and_b32_e32 v0, 0x7c00, v0
+; GFX7SELDAG-NEXT:    v_cmp_ne_u32_e32 vcc, 0, v0
+; GFX7SELDAG-NEXT:    v_cndmask_b32_e64 v0, 0, 1, vcc
 ; GFX7SELDAG-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX7GLISEL-LABEL: not_issubnormal_or_zero_f16:
 ; GFX7GLISEL:       ; %bb.0: ; %entry
 ; GFX7GLISEL-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7GLISEL-NEXT:    v_and_b32_e32 v0, 0x7fff, v0
 ; GFX7GLISEL-NEXT:    v_bfe_u32 v1, v0, 0, 16
 ; GFX7GLISEL-NEXT:    s_movk_i32 s4, 0x7c00

llvm/test/CodeGen/X86/is_fpclass.ll

 ; CHECK-32-LABEL: not_iszero_f:
 ; CHECK-32:       # %bb.0: # %entry
 ; CHECK-32-NEXT:    flds {{[0-9]+}}(%esp)
 ; CHECK-32-NEXT:    fldz
 ; CHECK-32-NEXT:    fucompp
 ; CHECK-32-NEXT:    fnstsw %ax
 ; CHECK-32-NEXT:    # kill: def $ah killed $ah killed $ax
 ; CHECK-32-NEXT:    sahf
-; CHECK-32-NEXT:    setp %cl
 ; CHECK-32-NEXT:    setne %al
-; CHECK-32-NEXT:    orb %cl, %al
 ; CHECK-32-NEXT:    retl
 ;
 ; CHECK-64-LABEL: not_iszero_f:
 ; CHECK-64:       # %bb.0: # %entry
 ; CHECK-64-NEXT:    xorps %xmm1, %xmm1
-; CHECK-64-NEXT:    cmpneqss %xmm0, %xmm1
-; CHECK-64-NEXT:    movd %xmm1, %eax
-; CHECK-64-NEXT:    andl $1, %eax
-; CHECK-64-NEXT:    # kill: def $al killed $al killed $eax
+; CHECK-64-NEXT:    ucomiss %xmm1, %xmm0
+; CHECK-64-NEXT:    setne %al
 ; CHECK-64-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 927)  ; ~0x60 = "~zero"
   ret i1 %0
 }
 
 define i1 @not_iszero_f_daz(float %x) #0 {
 ; CHECK-32-LABEL: not_iszero_f_daz:

 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 240)  ; 0xf0 = "subnormal|zero"
   ret i1 %0
 }
 
 define i1 @not_issubnormal_or_zero_f(float %x) {
 ; CHECK-32-LABEL: not_issubnormal_or_zero_f:
 ; CHECK-32:       # %bb.0: # %entry
-; CHECK-32-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; CHECK-32-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; CHECK-32-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; CHECK-32-NEXT:    setge %cl
-; CHECK-32-NEXT:    addl $-8388608, %eax # imm = 0xFF800000
-; CHECK-32-NEXT:    cmpl $2130706432, %eax # imm = 0x7F000000
-; CHECK-32-NEXT:    setb %al
-; CHECK-32-NEXT:    orb %cl, %al
+; CHECK-32-NEXT:    testl $2139095040, {{[0-9]+}}(%esp) # imm = 0x7F800000
+; CHECK-32-NEXT:    setne %al
 ; CHECK-32-NEXT:    retl
 ;
 ; CHECK-64-LABEL: not_issubnormal_or_zero_f:
 ; CHECK-64:       # %bb.0: # %entry
 ; CHECK-64-NEXT:    movd %xmm0, %eax
-; CHECK-64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; CHECK-64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; CHECK-64-NEXT:    setge %cl
-; CHECK-64-NEXT:    addl $-8388608, %eax # imm = 0xFF800000
-; CHECK-64-NEXT:    cmpl $2130706432, %eax # imm = 0x7F000000
-; CHECK-64-NEXT:    setb %al
-; CHECK-64-NEXT:    orb %cl, %al
+; CHECK-64-NEXT:    testl $2139095040, %eax # imm = 0x7F800000
+; CHECK-64-NEXT:    setne %al
 ; CHECK-64-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 783)  ; ~0xf0 = "~(subnormal|zero)"
   ret i1 %0
 }
 
 define i1 @not_issubnormal_or_zero_f_daz(float %x) #0 {
 ; CHECK-32-LABEL: not_issubnormal_or_zero_f_daz:
 ; CHECK-32:       # %bb.0: # %entry
-; CHECK-32-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; CHECK-32-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; CHECK-32-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; CHECK-32-NEXT:    setge %cl
-; CHECK-32-NEXT:    addl $-8388608, %eax # imm = 0xFF800000
-; CHECK-32-NEXT:    cmpl $2130706432, %eax # imm = 0x7F000000
-; CHECK-32-NEXT:    setb %al
-; CHECK-32-NEXT:    orb %cl, %al
+; CHECK-32-NEXT:    flds {{[0-9]+}}(%esp)
+; CHECK-32-NEXT:    fldz
+; CHECK-32-NEXT:    fucompp
+; CHECK-32-NEXT:    fnstsw %ax
+; CHECK-32-NEXT:    # kill: def $ah killed $ah killed $ax
+; CHECK-32-NEXT:    sahf
+; CHECK-32-NEXT:    setne %al
 ; CHECK-32-NEXT:    retl
 ;
 ; CHECK-64-LABEL: not_issubnormal_or_zero_f_daz:
 ; CHECK-64:       # %bb.0: # %entry
-; CHECK-64-NEXT:    movd %xmm0, %eax
-; CHECK-64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; CHECK-64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; CHECK-64-NEXT:    setge %cl
-; CHECK-64-NEXT:    addl $-8388608, %eax # imm = 0xFF800000
-; CHECK-64-NEXT:    cmpl $2130706432, %eax # imm = 0x7F000000
-; CHECK-64-NEXT:    setb %al
-; CHECK-64-NEXT:    orb %cl, %al
+; CHECK-64-NEXT:    xorps %xmm1, %xmm1
+; CHECK-64-NEXT:    ucomiss %xmm1, %xmm0
+; CHECK-64-NEXT:    setne %al
 ; CHECK-64-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 783)  ; ~0xf0 = "~(subnormal|zero)"
   ret i1 %0
 }
 
 define i1 @not_issubnormal_or_zero_f_maybe_daz(float %x) #1 {
 ; CHECK-32-LABEL: not_issubnormal_or_zero_f_maybe_daz:
 ; CHECK-32:       # %bb.0: # %entry
-; CHECK-32-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; CHECK-32-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; CHECK-32-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; CHECK-32-NEXT:    setge %cl
-; CHECK-32-NEXT:    addl $-8388608, %eax # imm = 0xFF800000
-; CHECK-32-NEXT:    cmpl $2130706432, %eax # imm = 0x7F000000
-; CHECK-32-NEXT:    setb %al
-; CHECK-32-NEXT:    orb %cl, %al
+; CHECK-32-NEXT:    testl $2139095040, {{[0-9]+}}(%esp) # imm = 0x7F800000
+; CHECK-32-NEXT:    setne %al
 ; CHECK-32-NEXT:    retl
 ;
 ; CHECK-64-LABEL: not_issubnormal_or_zero_f_maybe_daz:
 ; CHECK-64:       # %bb.0: # %entry
 ; CHECK-64-NEXT:    movd %xmm0, %eax
-; CHECK-64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; CHECK-64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; CHECK-64-NEXT:    setge %cl
-; CHECK-64-NEXT:    addl $-8388608, %eax # imm = 0xFF800000
-; CHECK-64-NEXT:    cmpl $2130706432, %eax # imm = 0x7F000000
-; CHECK-64-NEXT:    setb %al
-; CHECK-64-NEXT:    orb %cl, %al
+; CHECK-64-NEXT:    testl $2139095040, %eax # imm = 0x7F800000
+; CHECK-64-NEXT:    setne %al
 ; CHECK-64-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 783)  ; ~0xf0 = "~(subnormal|zero)"
   ret i1 %0
 }
 
 define i1 @is_plus_zero_f(float %x) {
 ; CHECK-32-LABEL: is_plus_zero_f: