[PATCH] D130311: [RISCV] Enable strict FP in clang as long as Zve* or V are not enabled.

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

I don't feel very qualified on the current state of strict FP, but I've left a couple of minor comments inline. I'm happy to trust your knowledge the state of strictfp and the patch looks good to me otherwise.



================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:286
+  // StrictFP support for vectors is incomplete.
+  if (ISAInfo->hasExtension("zve32x"))
+    HasStrictFP = false;
----------------
There's also code in RISCVISAInfo.cpp that does `HasVector = Exts.count("zve32x") != 0`. It's probably worth adding a helper (`hasVInstructions`?) that encapsulates this, and use it from both places.


================
Comment at: clang/test/CodeGen/builtin_float_strictfp.c:3
 // RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple ppc64-be -ffp-exception-behavior=maytrap -o - %s | FileCheck %s --check-prefixes=CHECK,NOFP16
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple riscv64 -ffp-exception-behavior=maytrap -o - %s | FileCheck %s --check-prefixes=CHECK,FP16
 
----------------
craig.topper wrote:
> I just picked a test to have some coverage that the strict FP is enabled. Most strict FP tests use `-Xclang -fexperimental-strict-floating-point` to avoid needing a specific target.
I think it would be worth at least having coverage for the logic that disables strict FP if vector extensions are enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130311