[PATCH] D88913: [FPEnv] Use strictfp metadata in casting nodes

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

I'll see how much I can simplify of the tests.



================
Comment at: clang/test/CodeGen/exprs-strictfp.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -ffp-exception-behavior=maytrap -emit-llvm -o - | FileCheck %s
----------------
sepavloff wrote:
> No FP casts.
There's an implied cast in the AST, and I'm pretty sure the bits from that are used when simplifying this into the fcmp. I'll doublecheck.


================
Comment at: clang/test/CodeGen/ubsan-conditional-strictfp.c:1
+// RUN: %clang_cc1 %s -triple x86_64-unknown-unknown -ffp-exception-behavior=maytrap -emit-llvm -fsanitize=float-divide-by-zero -o - | FileCheck %s
+
----------------
sepavloff wrote:
> What is the effect of `-fsanitize=float-divide-by-zero`? If it absents does the test change behavior?
An earlier version of this patch did try passing bits all the way down to the calls to the IRBuilder. In this version of the patch the sanitizer was required to be involved to test one of the places that was changed. I had 100% test coverage for that version of the code.

I'll check and see if it is still needed.


================
Comment at: clang/test/CodeGen/zvector-strictfp.c:9-23
+volatile vector signed char sc, sc2;
+volatile vector unsigned char uc, uc2;
+volatile vector bool char bc, bc2;
+
+volatile vector signed short ss, ss2;
+volatile vector unsigned short us, us2;
+volatile vector bool short bs, bs2;
----------------
sepavloff wrote:
> These are integer values, they cannot produce constrained intrinsics. Why they are tested here?
This test is also a holdover from the first version of the patch. It was also needed to achieve 100% test coverage with that first version. I'll check and see if it is still needed in this v2 version of the patch.


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

https://reviews.llvm.org/D88913