[PATCH] D141560: [RISCV][CodeGen] Add codegen pattern for experimental zfa extension

[Jun Sha via Phabricator via llvm-commits]

joshua-arch1 added a comment.

In D141560#4122948 <https://reviews.llvm.org/D141560#4122948>, @reames wrote:

> In D141560#4120971 <https://reviews.llvm.org/D141560#4120971>, @joshua-arch1 wrote:
>
>> Since the assmebly support patch for Zfa instructions except FLI has been accepted, could anyone please review this codegen patch?
>
> I'd encourage you to land the underlying approved review.  I can't speak for others, but I certainty prioritize review for changes which have fewer outstanding blocking issues.  You could also split this into an FLI and non-FLI part with the same reasoning.

Different from assembly support, I think there is nothing special with the codegen pattern for FLI instruction. It's unnecessory to split the codegen patch into an FLI and non-FLI part.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2004
+      if (SatVT == DstVT)
+        Opc = IsSigned ? RISCVISD::FCVTMOD_X : RISCVISD::FCVTMOD_XU;
+      else if (DstVT == MVT::i64 && SatVT == MVT::i32)
----------------
craig.topper wrote:
> FCVTMOD does not saturate. It returns the lower bits of the overflowed value.  These instruction cannot be used to lower saturating conversion.
So what's the difference between FCVTMOD and normal FP_TO_INT? Is NaN converted to zero in normal FP_TO_INT?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2004
+      if (SatVT == DstVT)
+        Opc = IsSigned ? RISCVISD::FCVTMOD_X : RISCVISD::FCVTMOD_XU;
+      else if (DstVT == MVT::i64 && SatVT == MVT::i32)
----------------
joshua-arch1 wrote:
> craig.topper wrote:
> > FCVTMOD does not saturate. It returns the lower bits of the overflowed value.  These instruction cannot be used to lower saturating conversion.
> So what's the difference between FCVTMOD and normal FP_TO_INT? Is NaN converted to zero in normal FP_TO_INT?
What instruction sequence does FCVTMOD need to replace? I'm a bit confused.


================
Comment at: llvm/test/CodeGen/RISCV/double-zfa.ll:108
+; RV64DZFA-NEXT:    ret
+  ret double 0x102F3E9DF9CF94
+}
----------------
craig.topper wrote:
> This doesn't look like the minimum value for normal value for double. I would expect the mantissa bits to be 0.
> 
> So it should be 0x0010000000000000 I think?
It is IR syntax. I have written a simple case to confirm that this value is used to express 0x0010000000000000 in IR level.


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

https://reviews.llvm.org/D141560