[PATCH] D101884: [WebAssembly] Fix constness of pointer params to load intrinsics

[Thomas Lively via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG602f318cfdac: [WebAssembly] Fix constness of pointer params 
to load intrinsics (authored by tlively).

Changed prior to commit:
  https://reviews.llvm.org/D101884?vs=342951=343158#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101884

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/Headers/wasm_simd128.h
  clang/test/CodeGen/builtins-wasm.c
  clang/test/Headers/wasm.c

Index: clang/test/Headers/wasm.c
===
--- clang/test/Headers/wasm.c
+++ clang/test/Headers/wasm.c
@@ -1,7 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --force-update
 // REQUIRES: webassembly-registered-target, asserts
 
-// RUN: %clang %s -O2 -emit-llvm -S -o - -target wasm32-unknown-unknown -msimd128 | FileCheck %s
+// RUN: %clang %s -O2 -emit-llvm -S -o - -target wasm32-unknown-unknown -msimd128 -Wcast-qual -Werror | FileCheck %s
 
 #include 
 
@@ -165,7 +165,7 @@
 // CHECK-NEXT:[[TMP2:%.*]] = bitcast <16 x i8> [[TMP1]] to <4 x i32>
 // CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
-v128_t test_v128_load8_lane(uint8_t *ptr, v128_t vec) {
+v128_t test_v128_load8_lane(const uint8_t *ptr, v128_t vec) {
   return wasm_v128_load8_lane(ptr, vec, 15);
 }
 
@@ -176,7 +176,7 @@
 // CHECK-NEXT:[[TMP2:%.*]] = bitcast <8 x i16> [[TMP1]] to <4 x i32>
 // CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
-v128_t test_v128_load16_lane(uint16_t *ptr, v128_t vec) {
+v128_t test_v128_load16_lane(const uint16_t *ptr, v128_t vec) {
   return wasm_v128_load16_lane(ptr, vec, 7);
 }
 
@@ -185,7 +185,7 @@
 // CHECK-NEXT:[[TMP0:%.*]] = tail call <4 x i32> @llvm.wasm.load32.lane(i32* [[PTR:%.*]], <4 x i32> [[VEC:%.*]], i32 3)
 // CHECK-NEXT:ret <4 x i32> [[TMP0]]
 //
-v128_t test_v128_load32_lane(uint32_t *ptr, v128_t vec) {
+v128_t test_v128_load32_lane(const uint32_t *ptr, v128_t vec) {
   return wasm_v128_load32_lane(ptr, vec, 3);
 }
 
@@ -196,7 +196,7 @@
 // CHECK-NEXT:[[TMP2:%.*]] = bitcast <2 x i64> [[TMP1]] to <4 x i32>
 // CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
-v128_t test_v128_load64_lane(uint64_t *ptr, v128_t vec) {
+v128_t test_v128_load64_lane(const uint64_t *ptr, v128_t vec) {
   return wasm_v128_load64_lane(ptr, vec, 1);
 }
 
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -284,28 +284,28 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 load8_lane(signed char *p, i8x16 v) {
+i8x16 load8_lane(const signed char *p, i8x16 v) {
   return __builtin_wasm_load8_lane(p, v, 0);
   // WEBASSEMBLY: tail call <16 x i8> @llvm.wasm.load8.lane(
   // WEBASSEMBLY-SAME: i8* %p, <16 x i8> %v, i32 0)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i16x8 load16_lane(short *p, i16x8 v) {
+i16x8 load16_lane(const short *p, i16x8 v) {
   return __builtin_wasm_load16_lane(p, v, 0);
   // WEBASSEMBLY: tail call <8 x i16> @llvm.wasm.load16.lane(
   // WEBASSEMBLY-SAME: i16* %p, <8 x i16> %v, i32 0)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i32x4 load32_lane(int *p, i32x4 v) {
+i32x4 load32_lane(const int *p, i32x4 v) {
   return __builtin_wasm_load32_lane(p, v, 0);
   // WEBASSEMBLY: tail call <4 x i32> @llvm.wasm.load32.lane(
   // WEBASSEMBLY-SAME: i32* %p, <4 x i32> %v, i32 0)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i64x2 load64_lane(long long *p, i64x2 v) {
+i64x2 load64_lane(const long long *p, i64x2 v) {
   return __builtin_wasm_load64_lane(p, v, 0);
   // WEBASSEMBLY: tail call <2 x i64> @llvm.wasm.load64.lane(
   // WEBASSEMBLY-SAME: i64* %p, <2 x i64> %v, i32 0)
@@ -904,13 +904,13 @@
   // WEBASSEMBLY: ret
 }
 
-i32x4 load32_zero(int *p) {
+i32x4 load32_zero(const int *p) {
   return __builtin_wasm_load32_zero(p);
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.load32.zero(i32* %p)
   // WEBASSEMBLY: ret
 }
 
-i64x2 load64_zero(long long *p) {
+i64x2 load64_zero(const long long *p) {
   return __builtin_wasm_load64_zero(p);
   // WEBASSEMBLY: call <2 x i64> @llvm.wasm.load64.zero(i64* %p)
   // WEBASSEMBLY: ret
Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -169,18 +169,19 @@
 }
 
 #define wasm_v128_load8_lane(__ptr, __vec, __i)\
-  ((v128_t)__builtin_wasm_load8_lane((signed char *)(__ptr), (__i8x16)(__vec), \
- (__i)))
+  ((v128_t)__builtin_wasm_load8_lane((const signed char *)(__ptr), \
+ (__i8x16)(__vec), (__i)))
 
 #define wasm_v128_load16_lane(__ptr, __vec, __i)   \
-  

[PATCH] D101884: [WebAssembly] Fix constness of pointer params to load intrinsics

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D101884#2739898 , @aheejin wrote:

> I see, thanks. Then other load instructions are OK not to be fixed?
>
> - `v128.load`
> - `v128.loadN_splat`
> - `v128.loadNxM_s/u`

Yep, the intrinsics for those instructions were already using const pointers 
correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101884

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101884: [WebAssembly] Fix constness of pointer params to load intrinsics

[Heejin Ahn via Phabricator via cfe-commits]

aheejin accepted this revision.
aheejin added a comment.
This revision is now accepted and ready to land.

I see, thanks. Then other load instructions are OK not to be fixed?

- `v128.load`
- `v128.loadN_splat`
- `v128.loadNxM_s/u`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101884

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101884: [WebAssembly] Fix constness of pointer params to load intrinsics

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

It only matters for pointer parameters. For example, `const int *` is 
meaningfully different from `int *`, but `const int` is not usefully different 
from `int`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101884

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101884: [WebAssembly] Fix constness of pointer params to load intrinsics

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

Why do only these instructions have `const`? It doesn't apply to other 
read-only arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101884

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101884: [WebAssembly] Fix constness of pointer params to load intrinsics

[Thomas Lively via Phabricator via cfe-commits]

tlively created this revision.
tlively added a reviewer: aheejin.
Herald added subscribers: wingo, ecnelises, sunfish, jgravelle-google, sbc100, 
dschuff.
tlively requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update the SIMD builtin load functions to take pointers to const data and update
the intrinsics themselves to not cast away constness.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101884

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/Headers/wasm_simd128.h
  clang/test/CodeGen/builtins-wasm.c
  clang/test/Headers/wasm.c

Index: clang/test/Headers/wasm.c
===
--- clang/test/Headers/wasm.c
+++ clang/test/Headers/wasm.c
@@ -1,7 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --force-update
 // REQUIRES: webassembly-registered-target
 
-// RUN: %clang %s -O2 -emit-llvm -S -o - -target wasm32-unknown-unknown -msimd128 | FileCheck %s
+// RUN: %clang %s -O2 -emit-llvm -S -o - -target wasm32-unknown-unknown -msimd128 -Wcast-qual -Werror | FileCheck %s
 
 #include 
 
@@ -165,7 +165,7 @@
 // CHECK-NEXT:[[TMP2:%.*]] = bitcast <16 x i8> [[TMP1]] to <4 x i32>
 // CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
-v128_t test_v128_load8_lane(uint8_t *ptr, v128_t vec) {
+v128_t test_v128_load8_lane(const uint8_t *ptr, v128_t vec) {
   return wasm_v128_load8_lane(ptr, vec, 15);
 }
 
@@ -176,7 +176,7 @@
 // CHECK-NEXT:[[TMP2:%.*]] = bitcast <8 x i16> [[TMP1]] to <4 x i32>
 // CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
-v128_t test_v128_load16_lane(uint16_t *ptr, v128_t vec) {
+v128_t test_v128_load16_lane(const uint16_t *ptr, v128_t vec) {
   return wasm_v128_load16_lane(ptr, vec, 7);
 }
 
@@ -185,7 +185,7 @@
 // CHECK-NEXT:[[TMP0:%.*]] = tail call <4 x i32> @llvm.wasm.load32.lane(i32* [[PTR:%.*]], <4 x i32> [[VEC:%.*]], i32 3)
 // CHECK-NEXT:ret <4 x i32> [[TMP0]]
 //
-v128_t test_v128_load32_lane(uint32_t *ptr, v128_t vec) {
+v128_t test_v128_load32_lane(const uint32_t *ptr, v128_t vec) {
   return wasm_v128_load32_lane(ptr, vec, 3);
 }
 
@@ -196,7 +196,7 @@
 // CHECK-NEXT:[[TMP2:%.*]] = bitcast <2 x i64> [[TMP1]] to <4 x i32>
 // CHECK-NEXT:ret <4 x i32> [[TMP2]]
 //
-v128_t test_v128_load64_lane(uint64_t *ptr, v128_t vec) {
+v128_t test_v128_load64_lane(const uint64_t *ptr, v128_t vec) {
   return wasm_v128_load64_lane(ptr, vec, 1);
 }
 
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -284,28 +284,28 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 load8_lane(signed char *p, i8x16 v) {
+i8x16 load8_lane(const signed char *p, i8x16 v) {
   return __builtin_wasm_load8_lane(p, v, 0);
   // WEBASSEMBLY: tail call <16 x i8> @llvm.wasm.load8.lane(
   // WEBASSEMBLY-SAME: i8* %p, <16 x i8> %v, i32 0)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i16x8 load16_lane(short *p, i16x8 v) {
+i16x8 load16_lane(const short *p, i16x8 v) {
   return __builtin_wasm_load16_lane(p, v, 0);
   // WEBASSEMBLY: tail call <8 x i16> @llvm.wasm.load16.lane(
   // WEBASSEMBLY-SAME: i16* %p, <8 x i16> %v, i32 0)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i32x4 load32_lane(int *p, i32x4 v) {
+i32x4 load32_lane(const int *p, i32x4 v) {
   return __builtin_wasm_load32_lane(p, v, 0);
   // WEBASSEMBLY: tail call <4 x i32> @llvm.wasm.load32.lane(
   // WEBASSEMBLY-SAME: i32* %p, <4 x i32> %v, i32 0)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i64x2 load64_lane(long long *p, i64x2 v) {
+i64x2 load64_lane(const long long *p, i64x2 v) {
   return __builtin_wasm_load64_lane(p, v, 0);
   // WEBASSEMBLY: tail call <2 x i64> @llvm.wasm.load64.lane(
   // WEBASSEMBLY-SAME: i64* %p, <2 x i64> %v, i32 0)
@@ -904,13 +904,13 @@
   // WEBASSEMBLY: ret
 }
 
-i32x4 load32_zero(int *p) {
+i32x4 load32_zero(const int *p) {
   return __builtin_wasm_load32_zero(p);
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.load32.zero(i32* %p)
   // WEBASSEMBLY: ret
 }
 
-i64x2 load64_zero(long long *p) {
+i64x2 load64_zero(const long long *p) {
   return __builtin_wasm_load64_zero(p);
   // WEBASSEMBLY: call <2 x i64> @llvm.wasm.load64.zero(i64* %p)
   // WEBASSEMBLY: ret
Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -169,18 +169,19 @@
 }
 
 #define wasm_v128_load8_lane(__ptr, __vec, __i)\
-  ((v128_t)__builtin_wasm_load8_lane((signed char *)(__ptr), (__i8x16)(__vec), \
- (__i)))
+  ((v128_t)__builtin_wasm_load8_lane((const signed char *)(__ptr), \
+ (__i8x16)(__vec), (__i)))
 
 #define wasm_v128_load16_lane(__ptr, __vec, __i)   \
-