[PATCH] D100596: [WebAssembly] Remove saturating fp-to-int target intrinsics

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:124-127
+  if (Subtarget->hasNontrappingFPToInt())
+    for (auto Op : {ISD::FP_TO_SINT_SAT, ISD::FP_TO_UINT_SAT})
+      for (auto T : {MVT::i32, MVT::i64})
+        setOperationAction(Op, T, Custom);
----------------
aheejin wrote:
> So far these have been expanded then?
That's right 👍 


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:2100
+                                TargetLowering::DAGCombinerInfo &DCI) {
+  auto &DAG = DCI.DAG;
+  assert(N->getOpcode() == ISD::CONCAT_VECTORS);
----------------
aheejin wrote:
> So even though you used `Builder.CreateShuffleVector` in Clang, do they appear as not `VECTOR_SHUFFLE` but `CONCAT_VECTORS` by the time they reach isel?
Right. LLVM IR has no concat_vectors instruction, but the pattern is recognized by SelectionDAGBuilder and turned into a concat_vectors SDNode.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:2130
+    return SDValue();
+  if (!!SplatValue)
+    return SDValue();
----------------
aheejin wrote:
> Nit: I think it is a little easier to read 
Will fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100596