[WebAssembly] Use functions instead of macros for const SIMD intrinsics

[tlively]

Summary

To improve hygiene, consistency, and usability, it would be good to replace all
the macro intrinsics in wasm_simd128.h with functions. The reason for using
macros in the first place was to enforce the use of constants for some arguments
using _Static_assert with __builtin_constant_p. This commit switches to
using functions and uses the __diagnose_if__ attribute rather than
_Static_assert to enforce constantness.

The remaining macro intrinsics cannot be made into functions until the builtin
functions they are implemented with can be replaced with normal code patterns
because the builtin functions themselves require that their arguments are
constants.

This commit also fixes a bug with the const_splat intrinsics in which the f32x4
and f64x2 variants were incorrectly producing integer vectors.

[aheejin]

To improve hygiene, consistency, and usability, it would be good to replace all
the macro intrinsics in wasm_simd128.h with functions. The reason for using
macros in the first place was to enforce the use of constants for some arguments
using _Static_assert with __builtin_constant_p. This commit switches to
using functions and uses the __diagnose_if__ attribute rather than
_Static_assert to enforce constantness.

So what prevented us from using functions when we were using _Static_assert and why is it now possible to use functions?

The remaining macro intrinsics cannot be made into functions until the builtin
functions they are implemented with can be replaced with normal code patterns
because the builtin functions themselves require that their arguments are
constants.

Why can't we also use __REQUIRE_CONSTANT there? Can't we call __REQUIRE_CONSTANT before we call builtins within intrinsic functions?

[tlively]

In D102018#2743818, @aheejin wrote:

So what prevented us from using functions when we were using _Static_assert and why is it now possible to use functions?

From inside a function, the parameter never looks like a constant because the function body does not know how the function will be used. Since the static_asserts would have been statements on the function body, they would not have been able to assert that the arguments to the function were constant. We were using macros to copy the static_asserts to each call site so they could see the actual arguments. In contrast, it looks like the diagnose_if attribute is evaluated separately for each call site and is able to see the actual arguments.

The remaining macro intrinsics cannot be made into functions until the builtin
functions they are implemented with can be replaced with normal code patterns
because the builtin functions themselves require that their arguments are
constants.

Why can't we also use __REQUIRE_CONSTANT there? Can't we call __REQUIRE_CONSTANT before we call builtins within intrinsic functions?

The new __REQUIRE_CONSTANT would be able to enforce that the intrinsic is only called with constant arguments, but that information is not propagated to the builtin call in the intrinsic's body. From the builtin's point of view inside the function body, its arguments are function parameters, so they may not be constant. One fix for this would have been to remove the part of the builtin definitions that requires the arguments to be constants, but that seemed more hacky than just keeping the original macros for a little bit longer.

clang/lib/Headers/wasm_simd128.h

 typedef int __i32x2 __attribute__((__vector_size__(8), __aligned__(8)));
 typedef unsigned int __u32x2
     __attribute__((__vector_size__(8), __aligned__(8)));
 
 #define __DEFAULT_FN_ATTRS                                                     \
   __attribute__((__always_inline__, __nodebug__, __target__("simd128"),        \
                  __min_vector_width__(128)))
 
-#define __REQUIRE_CONSTANT(e)                                                  \
-  _Static_assert(__builtin_constant_p(e), "Expected constant")
+#define __REQUIRE_CONSTANT(c)                                                  \
+  __attribute__((__diagnose_if__(!__builtin_constant_p(c),                     \
+                                 #c " must be constant", "error")))
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v128_load(const void *__mem) {
   // UB-free unaligned access copied from xmmintrin.h
   struct __wasm_v128_load_struct {
     __v128_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((const struct __wasm_v128_load_struct *)__mem)->__v;
 }
static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_make(float __c0,
   return (v128_t)(__f32x4){__c0, __c1, __c2, __c3};
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f64x2_make(double __c0,
                                                             double __c1) {
   return (v128_t)(__f64x2){__c0, __c1};
 }
 
-#define wasm_i8x16_const(__c0, __c1, __c2, __c3, __c4, __c5, __c6, __c7, __c8, \
-                         __c9, __c10, __c11, __c12, __c13, __c14, __c15)       \
-  __extension__({                                                              \
-    __REQUIRE_CONSTANT(__c0);                                                  \
-    __REQUIRE_CONSTANT(__c1);                                                  \
-    __REQUIRE_CONSTANT(__c2);                                                  \
-    __REQUIRE_CONSTANT(__c3);                                                  \
-    __REQUIRE_CONSTANT(__c4);                                                  \
-    __REQUIRE_CONSTANT(__c5);                                                  \
-    __REQUIRE_CONSTANT(__c6);                                                  \
-    __REQUIRE_CONSTANT(__c7);                                                  \
-    __REQUIRE_CONSTANT(__c8);                                                  \
-    __REQUIRE_CONSTANT(__c9);                                                  \
-    __REQUIRE_CONSTANT(__c10);                                                 \
-    __REQUIRE_CONSTANT(__c11);                                                 \
-    __REQUIRE_CONSTANT(__c12);                                                 \
-    __REQUIRE_CONSTANT(__c13);                                                 \
-    __REQUIRE_CONSTANT(__c14);                                                 \
-    __REQUIRE_CONSTANT(__c15);                                                 \
-    (v128_t)(__i8x16){__c0, __c1, __c2,  __c3,  __c4,  __c5,  __c6,  __c7,     \
-                      __c8, __c9, __c10, __c11, __c12, __c13, __c14, __c15};   \
-  })
-
-#define wasm_i16x8_const(__c0, __c1, __c2, __c3, __c4, __c5, __c6, __c7)       \
-  __extension__({                                                              \
-    __REQUIRE_CONSTANT(__c0);                                                  \
-    __REQUIRE_CONSTANT(__c1);                                                  \
-    __REQUIRE_CONSTANT(__c2);                                                  \
-    __REQUIRE_CONSTANT(__c3);                                                  \
-    __REQUIRE_CONSTANT(__c4);                                                  \
-    __REQUIRE_CONSTANT(__c5);                                                  \
-    __REQUIRE_CONSTANT(__c6);                                                  \
-    __REQUIRE_CONSTANT(__c7);                                                  \
-    (v128_t)(__i16x8){__c0, __c1, __c2, __c3, __c4, __c5, __c6, __c7};         \
-  })
-
-#define wasm_i32x4_const(__c0, __c1, __c2, __c3)                               \
-  __extension__({                                                              \
-    __REQUIRE_CONSTANT(__c0);                                                  \
-    __REQUIRE_CONSTANT(__c1);                                                  \
-    __REQUIRE_CONSTANT(__c2);                                                  \
-    __REQUIRE_CONSTANT(__c3);                                                  \
-    (v128_t)(__i32x4){__c0, __c1, __c2, __c3};                                 \
-  })
-
-#define wasm_i64x2_const(__c0, __c1)                                           \
-  __extension__({                                                              \
-    __REQUIRE_CONSTANT(__c0);                                                  \
-    __REQUIRE_CONSTANT(__c1);                                                  \
-    (v128_t)(__i64x2){__c0, __c1};                                             \
-  })
-
-#define wasm_f32x4_const(__c0, __c1, __c2, __c3)                               \
-  __extension__({                                                              \
-    __REQUIRE_CONSTANT(__c0);                                                  \
-    __REQUIRE_CONSTANT(__c1);                                                  \
-    __REQUIRE_CONSTANT(__c2);                                                  \
-    __REQUIRE_CONSTANT(__c3);                                                  \
-    (v128_t)(__f32x4){__c0, __c1, __c2, __c3};                                 \
-  })
-
-#define wasm_f64x2_const(__c0, __c1)                                           \
-  __extension__({                                                              \
-    __REQUIRE_CONSTANT(__c0);                                                  \
-    __REQUIRE_CONSTANT(__c1);                                                  \
-    (v128_t)(__f64x2){__c0, __c1};                                             \
-  })
-
-#define wasm_i8x16_const_splat(__c)                                            \
-  wasm_i8x16_const(__c, __c, __c, __c, __c, __c, __c, __c, __c, __c, __c, __c, \
-                   __c, __c, __c, __c)
-
-#define wasm_i16x8_const_splat(__c)                                            \
-  wasm_i16x8_const(__c, __c, __c, __c, __c, __c, __c, __c)
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_i8x16_const(int8_t __c0, int8_t __c1, int8_t __c2, int8_t __c3,
+                 int8_t __c4, int8_t __c5, int8_t __c6, int8_t __c7,
+                 int8_t __c8, int8_t __c9, int8_t __c10, int8_t __c11,
+                 int8_t __c12, int8_t __c13, int8_t __c14, int8_t __c15)
+    __REQUIRE_CONSTANT(__c0) __REQUIRE_CONSTANT(__c1) __REQUIRE_CONSTANT(__c2)
+        __REQUIRE_CONSTANT(__c3) __REQUIRE_CONSTANT(__c4)
+            __REQUIRE_CONSTANT(__c5) __REQUIRE_CONSTANT(__c6)
+                __REQUIRE_CONSTANT(__c7) __REQUIRE_CONSTANT(__c8)
+                    __REQUIRE_CONSTANT(__c9) __REQUIRE_CONSTANT(__c10)
+                        __REQUIRE_CONSTANT(__c11) __REQUIRE_CONSTANT(__c12)
+                            __REQUIRE_CONSTANT(__c13) __REQUIRE_CONSTANT(__c14)
+                                __REQUIRE_CONSTANT(__c15) {
+  return (v128_t)(__i8x16){__c0,  __c1,  __c2,  __c3, __c4,  __c5,
+                           __c6,  __c7,  __c8,  __c9, __c10, __c11,
+                           __c12, __c13, __c14, __c15};
+}
 
-#define wasm_i32x4_const_splat(__c) wasm_i32x4_const(__c, __c, __c, __c)
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_i16x8_const(int16_t __c0, int16_t __c1, int16_t __c2, int16_t __c3,
+                 int16_t __c4, int16_t __c5, int16_t __c6, int16_t __c7)
+    __REQUIRE_CONSTANT(__c0) __REQUIRE_CONSTANT(__c1) __REQUIRE_CONSTANT(__c2)
+        __REQUIRE_CONSTANT(__c3) __REQUIRE_CONSTANT(__c4)
+            __REQUIRE_CONSTANT(__c5) __REQUIRE_CONSTANT(__c6)
+                __REQUIRE_CONSTANT(__c7) {
+  return (v128_t)(__i16x8){__c0, __c1, __c2, __c3, __c4, __c5, __c6, __c7};
+}
 
-#define wasm_i64x2_const_splat(__c) wasm_i64x2_const(__c, __c)
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_i32x4_const(int32_t __c0, int32_t __c1, int32_t __c2, int32_t __c3)
+    __REQUIRE_CONSTANT(__c0) __REQUIRE_CONSTANT(__c1) __REQUIRE_CONSTANT(__c2)
+        __REQUIRE_CONSTANT(__c3) {
+  return (v128_t)(__i32x4){__c0, __c1, __c2, __c3};
+}
 
-#define wasm_f32x4_const_splat(__c) wasm_i32x4_const(__c, __c, __c, __c)
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_const(int64_t __c0,
+                                                             int64_t __c1)
+    __REQUIRE_CONSTANT(__c0) __REQUIRE_CONSTANT(__c1) {
+  return (v128_t)(__i64x2){__c0, __c1};
+}
 
-#define wasm_f64x2_const_splat(__c) wasm_i64x2_const(__c, __c)
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_f32x4_const(float __c0, float __c1, float __c2, float __c3)
+    __REQUIRE_CONSTANT(__c0) __REQUIRE_CONSTANT(__c1) __REQUIRE_CONSTANT(__c2)
+        __REQUIRE_CONSTANT(__c3) {
+  return (v128_t)(__f32x4){__c0, __c1, __c2, __c3};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f64x2_const(double __c0,
+                                                             double __c1)
+    __REQUIRE_CONSTANT(__c0) __REQUIRE_CONSTANT(__c1) {
+  return (v128_t)(__f64x2){__c0, __c1};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_const_splat(int8_t __c)
+    __REQUIRE_CONSTANT(__c) {
+  return (v128_t)(__i8x16){__c, __c, __c, __c, __c, __c, __c, __c,
+                           __c, __c, __c, __c, __c, __c, __c, __c};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i16x8_const_splat(int16_t __c)
+    __REQUIRE_CONSTANT(__c) {
+  return (v128_t)(__i16x8){__c, __c, __c, __c, __c, __c, __c, __c};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i32x4_const_splat(int32_t __c)
+    __REQUIRE_CONSTANT(__c) {
+  return (v128_t)(__i32x4){__c, __c, __c, __c};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_const_splat(int64_t __c)
+    __REQUIRE_CONSTANT(__c) {
+  return (v128_t)(__i64x2){__c, __c};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_const_splat(float __c)
+    __REQUIRE_CONSTANT(__c) {
+  return (v128_t)(__f32x4){__c, __c, __c, __c};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f64x2_const_splat(double __c)
+    __REQUIRE_CONSTANT(__c) {
+  return (v128_t)(__f64x2){__c, __c};
+}
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_splat(int8_t __a) {
   return (v128_t)(__i8x16){__a, __a, __a, __a, __a, __a, __a, __a,
                            __a, __a, __a, __a, __a, __a, __a, __a};
 }
 
 #define wasm_i8x16_extract_lane(__a, __i)                                      \
   (__builtin_wasm_extract_lane_s_i8x16((__i8x16)(__a), __i))

clang/test/Headers/wasm.c

 // CHECK-NEXT:    ret <4 x i32> <i32 42, i32 0, i32 42, i32 0>
 //
 v128_t test_i64x2_const_splat() {
   return wasm_i64x2_const_splat(42);
 }
 
 // CHECK-LABEL: @test_f32x4_const_splat(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:    ret <4 x i32> <i32 42, i32 42, i32 42, i32 42>
+// CHECK-NEXT:    ret <4 x i32> <i32 1109917696, i32 1109917696, i32 1109917696, i32 1109917696>
 //
 v128_t test_f32x4_const_splat() {
   return wasm_f32x4_const_splat(42);
 }
 
 // CHECK-LABEL: @test_f64x2_const_splat(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:    ret <4 x i32> <i32 42, i32 0, i32 42, i32 0>
+// CHECK-NEXT:    ret <4 x i32> <i32 0, i32 1078263808, i32 0, i32 1078263808>
 //
 v128_t test_f64x2_const_splat() {
   return wasm_f64x2_const_splat(42);
 }
 
 // CHECK-LABEL: @test_i8x16_splat(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[VECINIT_I:%.*]] = insertelement <16 x i8> undef, i8 [[A:%.*]], i32 0