[PATCH] D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui

Qing Shan Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 28 19:17:06 PDT 2020


steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:582
+    if (Subtarget.hasSPE()) {
+      setOperationAction(ISD::STRICT_FP_TO_UINT, MVT::i32, Legal);
       setOperationAction(ISD::FP_TO_UINT, MVT::i32, Legal);
----------------
jhibbits wrote:
> uweigand wrote:
> > jhibbits wrote:
> > > uweigand wrote:
> > > > uweigand wrote:
> > > > > jhibbits wrote:
> > > > > > nemanjai wrote:
> > > > > > > steven.zhang wrote:
> > > > > > > > Don’t do this for spe target and remove the test for spe. Sorry about the back and forth.
> > > > > > > I would defer to @jhibbits @chmeee (not sure which of Justin's ID's is active) regarding SPE bits.
> > > > > > What are the semantic differences between STRICT_FP_TO_UINT and FP_TO_UINT?  EFDCTUIZ/EFSCTUIZ and their signed counterparts, which we currently use for the FP_TO_{U,S}INT, saturate if they can't be represented as a 32-bit integer, and round toward zero always (the non-Z variants round via the current rounding mode).
> > > > > FP_TO_UINT assumes the current rounding mode is default, and exception conditions can be ignored.  With STRICT_FP_TO_UINT those assumptions no longer apply, so it would appear that those instructions you mention should not be used there.
> > > > Ah -- my last comment was incorrect, sorry for any confusion.
> > > > 
> > > > STRICT_FP_TO_UINT/SINT are in fact an exception to most "STRICT" operations in that they do **not** use the current rounding mode, but always round towards zero.  (Following the C standard as well as the LLVM IR specification.)
> > > > 
> > > > So for these operations the only difference between strict and non-strict variants is whether exception conditions can be ignored or not.
> > > Ah, okay.  So, if I understand correctly, EF{S,D}CT{S,U}I should be used for fp_to_{s,u}int, and the current 'Z' variants should be used for the strict_fp_to_*int.
> > Hmm, if I'm reading the SPE_PEM correctly, I think the "Z" variants are in fact correct for **both** strict and non-strict variants: they round towards zero (which both variants do), and they handle exceptions (which the strict variant requires, while the non-strict variant doesn't care).
> > 
> > The non-"Z" variants seem wrong either way since they use the current rounding mode, which is incorrect for both strict and non-strict variants.
> Thanks for that explanation @uweigand now I understand.  So the change here looks fine to me.
The reason why I asked to remove the spe here is to split this patch into two, one for PowerPC and another one for spe which need some inputs from spe experts. Does it make sense ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81537





More information about the llvm-commits mailing list