[PATCH] D132435: [RISCV] Fold fp_to_int(ftrunc (X)) -> fp_to_int(X) for vector.

[Jianjian Guan via Phabricator via llvm-commits]

jacquesguan added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8862
+  if (IsVector)
+    // (fp_to_int (ftrunc V))     -> fp_to_int (V)
+    // Keep fp_to_int for RVV, let it do custom lowerring.
----------------
craig.topper wrote:
> I'm not sure we should do this. There is a slight semantic difference. If this was legal to replace `(fp_to_int (ftrunc V))` with `fp_to_int (V)` we'd just do that in generic DAGCombine or InstCombine for all targets.
> 
> I need to think through exactly what bad thing can happen.
According to llvm langref:
'The `fptoui` instruction converts its floating-point operand into the nearest (rounding towards zero) unsigned integer value.'
And the `llvm.trunc.*` intrinsics is same with `trunc` of libm, which is descripted as : 'If no errors occur, the nearest integer value not greater in magnitude than arg (in other words, arg rounded towards zero), is returned.'

So I think they both are strict rounding towards zero. 

I tried common DAGCombiner at first, it would affect some other targets' test, so I switch to RISCV target specificed DAG combiner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132435