[PATCH] D137811: InstCombine: Port amdgcn.class intrinsic combines to is.fpclass

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:818-829
+  const ConstantInt *CMask = dyn_cast<ConstantInt>(Src1);
+  if (!CMask) {
+    if (isa<UndefValue>(Src0)) {
+      return replaceInstUsesWith(II, UndefValue::get(II.getType()));
+    }
+
+    if (isa<UndefValue>(Src1)) {
----------------
The second argument is described with `ImmArg`. It must always be a constant.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:842
+    // Equivalent of isnan. Replace with standard fcmp.
+    Value *FCmp = Builder.CreateFCmpUNO(Src0, Src0);
+    FCmp->takeName(&II);
----------------
This replacement is not valid in general case, only if FP exceptions are ignored. If the argument is a signaling NaN, compare instruction raises `Invalid` exception.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:849
+    // Equivalent of == 0.
+    Value *FCmp =
+        Builder.CreateFCmpOEQ(Src0, ConstantFP::get(Src0->getType(), 0.0));
----------------
It also is not always valid, only if the argument is not a signaling NaN or FP exceptions are ignored..


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:881
+
+  const APFloat &Val = CVal->getValueAPF();
+
----------------
Constant folding should be done in `ConstantFolding.cpp`.


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

https://reviews.llvm.org/D137811