aheejin added inline comments.
================
Comment at: include/clang/Basic/BuiltinsWebAssembly.def:53
// Saturating fp-to-int conversions
-BUILTIN(__builtin_wasm_trunc_saturate_s_i32_f32, "if", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_u_i32_f32, "if", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_s_i32_f64, "id", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_u_i32_f64, "id", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_s_i64_f32, "LLif", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_u_i64_f32, "LLif", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_s_i64_f64, "LLid", "nc")
-BUILTIN(__builtin_wasm_trunc_saturate_u_i64_f64, "LLid", "nc")
-
-// Floating point min/max
-BUILTIN(__builtin_wasm_min_f32, "fff", "nc")
-BUILTIN(__builtin_wasm_max_f32, "fff", "nc")
-BUILTIN(__builtin_wasm_min_f64, "ddd", "nc")
-BUILTIN(__builtin_wasm_max_f64, "ddd", "nc")
+TARGET_BUILTIN(__builtin_wasm_trunc_saturate_s_i32_f32, "if", "nc",
"nontrapping-fptoint")
+TARGET_BUILTIN(__builtin_wasm_trunc_saturate_u_i32_f32, "if", "nc",
"nontrapping-fptoint")
----------------
clang-format this file
================
Comment at: lib/Basic/Targets/WebAssembly.cpp:89
+ // features control availability of builtins
+ setSIMDLevel(Features, SIMDLevel);
+ if (HasNontrappingFPToInt)
----------------
Minor thing, but should we extract this as a function? It is gonna be a couple
line anyway, like
```
if (CPU == "bleeding-edge") {
...
Features["unimplemented-simd128"] = Features["simd128"] = true;
}
if (SIMDLevel >= SIMD128)
Features["simd128"] = true;
if (SIMDLevel >= UnimplementedSIMD128)
Features["unimplemented-simd128"] = true;
...
And to me it is more readable to see all `Features` setting in one place. But
I'm not too opinionated either.
================
Comment at: lib/Basic/Targets/WebAssembly.cpp:98
+ return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
+ }
+
----------------
The indentation of these functions looks weird and there are lines that exceeds
80 cols. clang-format?
================
Comment at: test/CodeGen/builtins-wasm.c:2
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -target-feature
+unimplemented-simd128 -target-feature +nontrapping-fptoint -target-feature
+exception-handling -fno-lax-vector-conversions -O3 -emit-llvm -o - %s |
FileCheck %s -check-prefixes WEBASSEMBLY,WEBASSEMBLY32
+// RUN: %clang_cc1 -triple wasm64-unknown-unknown -target-feature
+unimplemented-simd128 -target-feature +nontrapping-fptoint -target-feature
+exception-handling -fno-lax-vector-conversions -O3 -emit-llvm -o - %s |
FileCheck %s -check-prefixes WEBASSEMBLY,WEBASSEMBLY64
----------------
Maybe we can add a line that disables one of the target features and make sure
it fails? You can do something like
```
RUN: not %clang_cc1 ...
```
to see if it fails.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56504/new/
https://reviews.llvm.org/D56504
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
