[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

Sam Clegg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 10 17:47:38 PDT 2020


sbc100 added inline comments.


================
Comment at: clang/lib/Basic/Targets/WebAssembly.h:33
 
-  bool HasNontrappingFPToInt = false;
+  bool HasNontrappingFPToInt = true;
   bool HasSignExt = false;
----------------
tlively wrote:
> It seems strange to change the default here. x86 initializes all its corresponding features to `false` then selectively enables them in `initFeatureMap`. I think we should stick with that pattern if possible.
Right.. makes sense. Done.


================
Comment at: clang/test/Preprocessor/wasm-target-features.c:10
+
+// NONTRAPPING-FPTOINT:#define __wasm_nontrapping_fptoint__ 1{{$}}
+
----------------
tlively wrote:
> Is there a RUN line corresponding to this check anymore?
Oops, deleted that line.


================
Comment at: clang/test/Preprocessor/wasm-target-features.c:37
 //
-// NONTRAPPING-FPTOINT:#define __wasm_nontrapping_fptoint__ 1{{$}}
+// NO-NONTRAPPING-FPTOINT-NOT:#define __wasm_nontrapping_fptoint__ 1{{$}}
 
----------------
Here is t he test for disabling in clang.

For llc there was already a test for `-mattr=-nontrapping-fptoint` in the form of `llvm/test/CodeGen/WebAssembly/conv-trap.ll`.   This is what caused me to need to fix `getFeatureString`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77908





More information about the cfe-commits mailing list