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

225–226

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

866–868

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

866–868

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

866–868

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

866–868

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

8113

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

8113

I don't really remember what I was thinking here

[sepavloff]

LGTM.

llvm/lib/CodeGen/CodeGenCommonISel.cpp

177

[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 InvertedTest = ~Test & fcAllFlags;
-  switch (InvertedTest) {
-  default:
-    break;
+FPClassTest llvm::invertFPClassTestIfSimpler(FPClassTest Test) {
+  FPClassTest InvertedTest = static_cast<FPClassTest>(~Test & fcAllFlags);
+  // 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 bool isFCmpEqualZero(FPClassTest Test, const fltSemantics &Semantics,
                             const MachineFunction &MF) {
+  // TODO: Handle unordered compares
   if (Test == fcZero &&
       MF.getDenormalMode(Semantics).Input == DenormalMode::IEEE)
     return true;
   if (Test == (fcZero | fcSubnormal) &&
       MF.getDenormalMode(Semantics).inputsAreZero())
     return true;
   return false;
 }
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(Test)) {
+  if (FPClassTest InvertedCheck = invertFPClassTestIfSimpler(Test)) {
     IsInverted = true;
     Test = 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() &&
       isOperationLegalOrCustom(ISD::SETCC, OperandVT.getScalarType())) {
     if (isFCmpEqualZero(Test, Semantics, DAG.getMachineFunction()) &&
         (isCondCodeLegalOrCustom(IsInverted ? ISD::SETUNE : ISD::SETOEQ,
                                  OperandVT.getScalarType().getSimpleVT()))) {
       // 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);
     }
 
     if (Test == fcNan &&
         isCondCodeLegalOrCustom(IsInverted ? ISD::SETO : ISD::SETUO,
                                 OperandVT.getScalarType().getSimpleVT())) {
       return DAG.getSetCC(DL, ResultVT, Op, Op,
                           IsInverted ? ISD::SETO : ISD::SETUO);
     }
else if ((Test & fcFinite) == fcFinite) {
     Test &= ~fcNegFinite;
   }
   appendResult(PartialRes);
 
   if (FPClassTest 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_and_b32_e32 v1, 0xffff, v0
 ; GFX7GLISEL-NEXT:    s_movk_i32 s4, 0x7c00

llvm/test/CodeGen/X86/is_fpclass.ll

 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:    testl $2139095040, {{[0-9]+}}(%esp) # imm = 0x7F800000
+; 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:    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_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: