mibintc added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2988
CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
- // FIXME: for strictfp/IEEE-754 we need to not trap on SNaN here.
Value *V = EmitScalarExpr(E->getArg(0));
----------------
What about all the other FIXME related to strictfp, are we going to pick them
off piecemeal? It would be nice to have a holistic approach, it would be more
clear, and less duplicated code. For example, presumably the test at line 2991
is common.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3001
+ // NaN has all exp bits set and a non zero significand. Therefore:
+ // isnan(V) == ((abs(V) & exp mask) - exp mask < 0)
+ unsigned bitsize = Ty->getScalarSizeInBits();
----------------
Are you using a reference (e.g. existing implementation) for this rewrite, or
is this invention? If a reference can you please tell me what it is. The
expression you have written here doesn't match the codegen below. I don't see
the comparison to zero. Can you provide full parenthesization--the compare to
zero is lower than subtract?
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3010
+ Value *Sub =
+ Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+ V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));
----------------
compared to the comment above at line 3001, lhs and rhs are swapped in the sub
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3011
+ Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+ V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));
+ if (bitsize > 32)
----------------
the comment at line 3001 doesn't show the lshr or the compare to zero
================
Comment at: clang/test/CodeGen/X86/strictfp_builtins.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap -o - -triple
x86_64-unknown-unknown | FileCheck %s
----------------
Why did you put long double into this new test case instead of putting it with
the float and double in strictfp_builtins?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95948/new/
https://reviews.llvm.org/D95948
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
