Turn off fp reassociation in IA intrinsic header files.

[kbsmith1]

Summary

This change uses #pragma float_control and
pragma clang fp reassociate(off) directives to turn off
floating point fast-math reassociation flags for IA intrinsics
that happen to be implemented using LLVM IR's regular FP
operations.

This change was done to prevent reassociation from happening
across separate intrinsic calls, an optimization that looks like something
an intrinsic programmer would not expect.

[lebedev.ri]

Patch description states what the change is, but not why the change is what it is.
I don't think this is a good direction to second-guess the user's intentions.

[kbsmith1]

In D107004#2911654, @lebedev.ri wrote:

Patch description states what the change is, but not why the change is what it is.
I don't think this is a good direction to second-guess the user's intentions.

I added the reason for the change to the description. I think when a user programs
t = _mm_add_ps(a, b);
t = _mm_sub_ps(t, b);

that they truly did want those intrinsics evaluated like they were written, even with
fast math flags. This change to the intrinsic headers makes programming with intrinsics
more accurately reflect what the user wrote, even when fast math flags are used.

[craig.topper]

clang/lib/Headers/emmintrin.h

13

inrinsics->intrinsics

clang/lib/Headers/xmmintrin.h

13

inrinsics->intrinsics

[RKSimon]

I really don't support this change - -ffast-math should be allowed to perform aggressive folds and it makes no sense to make an exception for common intrinsics like these.

We should just ensure the intrinsics are suitably documented to warn that they are affected by fast math flags like any other operation.

An alternative, if you have specific (and common) use cases, would be to add an opt-in command line switch that disables fast math flags on intrinsics.

[kbsmith1]

Doesn't seem to have community support, abandoning.

clang/lib/Headers/emmintrin.h

 /*===---- emmintrin.h - SSE2 intrinsics ------------------------------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __EMMINTRIN_H
 #define __EMMINTRIN_H
 
+/* Turn off reassociation for inrinsics, push state to restore at end. */
+#pragma float_control(push)
+#pragma clang fp reassociate(off)
+
 #include <xmmintrin.h>
 
 typedef double __m128d __attribute__((__vector_size__(16), __aligned__(16)));
 typedef long long __m128i __attribute__((__vector_size__(16), __aligned__(16)));
 
 typedef double __m128d_u __attribute__((__vector_size__(16), __aligned__(1)));
 typedef long long __m128i_u __attribute__((__vector_size__(16), __aligned__(1)));
 

 #define _MM_DENORMALS_ZERO_ON   (0x0040U)
 #define _MM_DENORMALS_ZERO_OFF  (0x0000U)
 
 #define _MM_DENORMALS_ZERO_MASK (0x0040U)
 
 #define _MM_GET_DENORMALS_ZERO_MODE() (_mm_getcsr() & _MM_DENORMALS_ZERO_MASK)
 #define _MM_SET_DENORMALS_ZERO_MODE(x) (_mm_setcsr((_mm_getcsr() & ~_MM_DENORMALS_ZERO_MASK) | (x)))
 
+#pragma float_control(pop)
+
 #endif /* __EMMINTRIN_H */

clang/lib/Headers/immintrin.h

 /*===---- immintrin.h - Intel intrinsics -----------------------------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __IMMINTRIN_H
 #define __IMMINTRIN_H
 
+/* Turn off reassociation for inrinsics, push state to restore at end. */
+#pragma float_control(push)
+#pragma clang fp reassociate(off)
+
 #include <x86gprintrin.h>
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||      \
     defined(__MMX__)
 #include <mmintrin.h>
 #endif
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||      \

 #ifdef __cplusplus
 }
 #endif
 
 #undef __DEFAULT_FN_ATTRS
 
 #endif /* defined(_MSC_VER) && __has_extension(gnu_asm) */
 
+#pragma float_control(pop)
+
 #endif /* __IMMINTRIN_H */

clang/lib/Headers/x86intrin.h

 /*===---- x86intrin.h - X86 intrinsics -------------------------------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __X86INTRIN_H
 #define __X86INTRIN_H
 
+/* Turn off reassociation for inrinsics, push state to restore at end. */
+#pragma float_control(push)
+#pragma clang fp reassociate(off)
+
 #include <ia32intrin.h>
 
 #include <immintrin.h>
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||      \
     defined(__3dNOW__)
 #include <mm3dnow.h>
 #endif

 #include <mwaitxintrin.h>
 #endif
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||      \
     defined(__CLZERO__)
 #include <clzerointrin.h>
 #endif
 
+#pragma float_control(pop)
 
 #endif /* __X86INTRIN_H */

clang/lib/Headers/xmmintrin.h

 /*===---- xmmintrin.h - SSE intrinsics -------------------------------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __XMMINTRIN_H
 #define __XMMINTRIN_H
 
+/* Turn off reassociation for inrinsics, push state to restore at end. */
+#pragma float_control(push)
+#pragma clang fp reassociate(off)
+
 #include <mmintrin.h>
 
 typedef int __v4si __attribute__((__vector_size__(16)));
 typedef float __v4sf __attribute__((__vector_size__(16)));
 typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
 
 typedef float __m128_u __attribute__((__vector_size__(16), __aligned__(1)));
 

 #undef __DEFAULT_FN_ATTRS
 #undef __DEFAULT_FN_ATTRS_MMX
 
 /* Ugly hack for backwards-compatibility (compatible with gcc) */
 #if defined(__SSE2__) && !__building_module(_Builtin_intrinsics)
 #include <emmintrin.h>
 #endif
 
+#pragma float_control(pop)
+
 #endif /* __XMMINTRIN_H */

clang/test/Headers/emmintrin.c

+// RUN: %clang_cc1 %s -ffreestanding -ffast-math -triple x86_64-apple-macosx10.9.0 -emit-llvm -o - \
+// RUN:     | FileCheck -check-prefix=CKFMATH %s
+//
+#include <emmintrin.h>
+
+// Make sure that the llvm IR for _mm_add_pd doesn't have fast attribute and
+// doesn't have reassoc set.
+// CKFMATH: define{{.*}} <2 x double> @test_emmintrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn
+__m128d test_emmintrin_no_reassoc(__m128d __a, __m128d __b) {
+  return _mm_add_pd(__a, __b);
+}
+
+// Make sure that all fast flags were restored outside of the include file.
+// CKFMATH: define{{.*}} double @test_fast
+// CKFMATH: fadd reassoc nnan ninf nsz arcp afn double
+double test_fast(double __a, double __b) {
+  return __a + __b;
+}

clang/test/Headers/immintrin.c

+// RUN: %clang_cc1 %s -ffreestanding -ffast-math -triple i386-unknown-unknown -emit-llvm -o - \
+// RUN:     | FileCheck -check-prefix=CKFMATH %s
+//
+// RUN: %clang_cc1 %s -ffreestanding -ffast-math -triple x86_64-unknown-unknown -emit-llvm -o - \
+// RUN:     | FileCheck -check-prefix=CKFMATH %s
+
+// Include the metaheader that includes all intel intrinsic headers.
+#include <immintrin.h>
+
+// Make sure that the llvm IR for _mm_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// CKFMATH: define{{.*}} <4 x float> @test_xmmintrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <4 x float>
+__m128 __attribute__((__target__("sse"))) test_xmmintrin_no_reassoc(__m128 __a, __m128 __b) {
+  return _mm_add_ps(__a, __b);
+}
+
+// Make sure that the llvm IR for _mm_add_pd doesn't have fast attribute and
+// doesn't have reassoc set.
+// CKFMATH: define{{.*}} <2 x double> @test_emmintrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <2 x double>
+__m128d __attribute__((__target__("sse2"))) test_emmintrin_no_reassoc(__m128d __a, __m128d __b) {
+  return _mm_add_pd(__a, __b);
+}
+
+// Make sure that the llvm IR for _mm256_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// This intrinsic comes from avxintrin.h, and so is checking that
+// changes in immintrin.h affect the files it includes as well.
+// CKFMATH: define{{.*}} <8 x float> @test_mm256intrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <8 x float>
+__m256 __attribute__((__target__(("avx")))) test_mm256intrin_no_reassoc(__m256 __a, __m256 __b) {
+  return _mm256_add_ps(__a, __b);
+}
+
+// Make sure that the llvm IR for _mm512_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// This intrinsic comes from avxintrin.h
+// CKFMATH: define{{.*}} <16 x float> @test_mm512intrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <16 x float>
+__m512 __attribute__((__target__(("avx512f")))) test_mm512intrin_no_reassoc(__m512 __a, __m512 __b) {
+  return _mm512_add_ps(__a, __b);
+}
+
+// Make sure that all fast flags were restored outside of the include file.
+// CKFMATH: define{{.*}} double @test_fast
+// CKFMATH: fadd reassoc nnan ninf nsz arcp afn double
+double test_fast(double __a, double __b) {
+  return __a + __b;
+}

clang/test/Headers/x86intrin-3.c

+// RUN: %clang_cc1 %s -ffreestanding -ffast-math -triple i386-unknown-unknown -emit-llvm -o - \
+// RUN:     | FileCheck -check-prefix=CKFMATH %s
+//
+// RUN: %clang_cc1 %s -ffreestanding -ffast-math -triple x86_64-unknown-unknown -emit-llvm -o - \
+// RUN:     | FileCheck -check-prefix=CKFMATH %s
+
+// Include the metaheader that includes all x86 intrinsic headers.
+#include <x86intrin.h>
+
+// Make sure that the llvm IR for _mm_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// CKFMATH: define{{.*}} <4 x float> @test_xmmintrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <4 x float>
+__m128 __attribute__((__target__("sse"))) test_xmmintrin_no_reassoc(__m128 __a, __m128 __b) {
+  return _mm_add_ps(__a, __b);
+}
+
+// Make sure that the llvm IR for _mm_add_pd doesn't have fast attribute and
+// doesn't have reassoc set.
+// CKFMATH: define{{.*}} <2 x double> @test_emmintrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <2 x double>
+__m128d __attribute__((__target__("sse2"))) test_emmintrin_no_reassoc(__m128d __a, __m128d __b) {
+  return _mm_add_pd(__a, __b);
+}
+
+// Make sure that the llvm IR for _mm256_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// This intrinsic comes from avxintrin.h, and so is checking that
+// changes in immintrin.h affect the files it includes as well.
+// CKFMATH: define{{.*}} <8 x float> @test_mm256intrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <8 x float>
+__m256 __attribute__((__target__(("avx")))) test_mm256intrin_no_reassoc(__m256 __a, __m256 __b) {
+  return _mm256_add_ps(__a, __b);
+}
+
+// Make sure that the llvm IR for _mm512_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// This intrinsic comes from avxintrin.h
+// CKFMATH: define{{.*}} <16 x float> @test_mm512intrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <16 x float>
+__m512 __attribute__((__target__(("avx512f")))) test_mm512intrin_no_reassoc(__m512 __a, __m512 __b) {
+  return _mm512_add_ps(__a, __b);
+}
+
+// Make sure that all fast flags were restored outside of the include file.
+// CKFMATH: define{{.*}} double @test_fast
+// CKFMATH: fadd reassoc nnan ninf nsz arcp afn double
+double test_fast(double __a, double __b) {
+  return __a + __b;
+}

clang/test/Headers/xmmintrin.c

 // RUN: %clang_cc1 %s -ffreestanding -triple x86_64-apple-macosx10.9.0 -emit-llvm -o - | FileCheck %s
 //
+// RUN: %clang_cc1 %s -ffreestanding -ffast-math -triple x86_64-apple-macosx10.9.0 -emit-llvm -o - \
+// RUN:     | FileCheck -check-prefix=CKFMATH %s
+//
 // RUN: rm -rf %t
 // RUN: %clang_cc1 %s -ffreestanding -triple x86_64-apple-macosx10.9.0 -emit-llvm -o - \
 // RUN:     -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -isystem %S/Inputs/include \
 // RUN:     | FileCheck %s
 // REQUIRES: x86-registered-target
 #include <xmmintrin.h>
 
 // CHECK: @c ={{.*}} global i8 0, align 16

 }
 
 // Make sure that including <xmmintrin.h> also makes <emmintrin.h>'s content available.
 // This is an ugly hack for GCC compatibility.
 __m128d test_xmmintrin_provides_emmintrin(__m128d __a, __m128d __b) {
   return _mm_add_sd(__a, __b);
 }
 
+// Make sure that the llvm IR for _mm_add_ps doesn't have fast attribute and
+// doesn't have reassoc set.
+// CKFMATH: define{{.*}} <4 x float> @test_xmmintrin_no_reassoc
+// CKFMATH: fadd nnan ninf nsz arcp afn <4 x float>
+__m128 test_xmmintrin_no_reassoc(__m128 __a, __m128 __b) {
+  return _mm_add_ps(__a, __b);
+}
+
+// Make sure that all fast flags were restored outside of the include file.
+// CKFMATH: define{{.*}} double @test_fast
+// CKFMATH: fadd reassoc nnan ninf nsz arcp afn double
+double test_fast(double __a, double __b) {
+  return __a + __b;
+}
+
 #if __STDC_HOSTED__
 // Make sure stdlib.h symbols are accessible.
 void *p = NULL;
 #endif