[PATCH] D91972: Improve STRICT_FSETCC codegen in absence of no NaN

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre marked an inline comment as done.
thopre added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7129
+    ISD::CondCode Condition = getFCmpCondCode(FPCmp->getPredicate());
+    if (TM.Options.NoNaNsFPMath)
+      Condition = getFCmpCodeWithoutNaN(Condition);
----------------
SjoerdMeijer wrote:
> Nit: perhaps just make this an if-else.
I'm not sure what you mean, something like:


```
ISD::CondCode Condition = getFcmpCondCode(FPCmp->getPredicate());
if (TM.Options.NoNaNsFPMath)
  Opers.push_back(DAG.getCondCode(getFCmpCodeWithoutNaN(Condition)));
else
  Opers.push_back(DAG>getCondCode(Condition));
```

? I'd prefer to keep the current code since it mirrors what is done in visitFcmp and reflect better that even if a better condition code can be used (due to no NaN) we are still going to push it. YMMV of course.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-constrained-fcmp-no-nans-opt.ll:3
+
+declare i1 @llvm.experimental.constrained.fcmp.f32(float, float, metadata, metadata)
+
----------------
SjoerdMeijer wrote:
> Do we need to add tests for .f16 and .f64 just for completeness?  
I've only done the f64 because f16 does not appear to be supported on AArch64:


```
LLVM ERROR: Cannot select: 0x55d983a0fc58: f16,ch = AArch64ISD::STRICT_FCMP 0x55d9839a8938, 0x55d983a0fab8, 0x55d983a0fb88
  0x55d983a0fab8: f16,ch = CopyFromReg 0x55d9839a8938, Register:f16 %0
    0x55d983a0fa50: f16 = Register %0
  0x55d983a0fb88: f16,ch = CopyFromReg 0x55d9839a8938, Register:f16 %1
    0x55d983a0fb20: f16 = Register %1
In function: f16_constrained_fcmp_ueq
```

Happy to add it later if someone adds that support.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91972/new/

https://reviews.llvm.org/D91972