[PATCH] D140467: [X86][Reduce] Preserve fast math flags when change it. NFCI

Phoebe Wang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 22 07:04:01 PST 2022


pengfei updated this revision to Diff 484826.
pengfei added a comment.

Add test case to check FastMathFlagGuard works.

> Tests don't exist for users, they exist for compiler developers...
> I agree with @arsenm. At least for clang irgen, we should have good test coverage.

You are right. Added a test case.

> This test amounts to a builtin header test for immintrin.h. This should move to clang/test/Headers and replaced with a builtin test directly checking the builtin handling

I got your point now, thanks! However, I don't agree with you. Although most intrinsics are simple wrappers of builtins, we have many intrinsics are combinations of more builtins. It's common in mask intrinsics. We also have intrinsics that permute their arguments order when calling to builtins.
IIUC, `-fsyntax-only` only checks for types, builtins existence. It even doesn't check if intrinsics mapped to the right builtins. Not to mention above cases. So it doesn't sound feasible to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140467

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-x86-reduce.c


Index: clang/test/CodeGen/builtins-x86-reduce.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/builtins-x86-reduce.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-unknown -target-feature +avx512f -emit-llvm -o - | FileCheck %s
+
+typedef double double8 __attribute__((ext_vector_type(8)));
+
+double foo(double8 a, double b) {
+  return __builtin_ia32_reduce_fmax_pd512(a) + b;
+}
+
+// CHECK: fadd
+// CHECK-NOT: nnan
+// CHECK-SAME: double
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13113,6 +13113,8 @@
     return Builder.CreateBitCast(Sext, FPVecTy);
   };
 
+  IRBuilder<>::FastMathFlagGuard FMFGuard(Builder);
+
   switch (BuiltinID) {
   default: return nullptr;
   case X86::BI_mm_prefetch: {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140467.484826.patch
Type: text/x-patch
Size: 916 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221222/debb6bc4/attachment-0001.bin>


More information about the cfe-commits mailing list