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

[Qiu Chaofan via Phabricator via llvm-commits]

qiucf added a comment.

In D81537#2193216 <https://reviews.llvm.org/D81537#2193216>, @uweigand wrote:

> This doesn't look correct.  As far as I can see, none of the conversion functions were actually changed to handle strict operations.  For one, you'll need strict variants of all the PowerPC-specific conversion operations, use them in all the conversion subroutines, and consistently track their chain nodes.
>
> The patch only adds a strict variant of the direct move, which seems to me the only operation where actually a strict version is **not** required ...

Thanks for pointing them out! I have something unclear about chains:

(1) If a constrained operation is expanded into several FP nodes `a-b-c`, they should all have chain set to former operation (`b`'s chain is `a`, `c`'s chain is `b`) even if they have def relationship?

(2) In `MachineInstr` emitting after ISel, chains are identified just by operand type (countOperand <https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp#L62-L68>), so some chains are not ignored and assert hit. Is this expected?



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8307
+    return DAG.getNode(PPCISD::MFVSR, dl, Op.getValueType(),
+                       convertFPToInt(Op, DAG, Subtarget));
 }
----------------
uweigand wrote:
> Why do we need a strict version of a plain move?
Yes, a strict move doesn't look reasonable. But the original `strict_fptosi` node will be replaced by the result. So if directly return the move, operands will not match (no chain in `mfvsr`). Is there a better way here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81537