[PATCH] D70226: Add an option to disable strict float node mutating to an normal float node

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added a subscriber: kpn.
pengfei added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2668
+    return false;
+  }
+
----------------
LiuChen3 wrote:
> uweigand wrote:
> > I'm not sure this is always correct, I think it might be possible that a target might want to select Expand for a strict operation even if they use e.g. Custom for the non-strict version (obviously, that would have to be an operation where common code implements an Expand algorithm that respects the constrained FP semantics).
> > 
> > More importantly, even if you do this, you **still** need to add the checks in STRICT_FP_ROUND and STRICT_FP_EXTEND I mentioned in my earlier comment: note that in those cases, even if the target uses Expand for both the strict and non-strict operation, the code below still cannot be used if isStrictFPEnabled is true (since it does not respect constrained FP semantics).
> Thanks. 
> I think I misunderstood what you meant before. You actually mean is if the backend has supported strict float, it can never expand STRICT_FP_ROUND and STRICT_FP_EXTEND operations. We don't expand it not because we setOperationAction wrong or something else, because it isn't 'strict float' at all.
> I'll only add judgment based on the previous patch and delete this.
In my opinion, the behavior currently is reasonable. I don't think there's a way in common code can handle an expand strict node if its non-strict node is custom. Otherwise, its non-strict node isn't necessarily to be custom.
For STRICT_FP_ROUND and STRICT_FP_EXTEND, I reviewed the discussion between you and @kpn in D65226. I think it's equal to your change if the action of the target's non-strict nodes is legal. And for target that isStrictFPEnabled, the action of the strict nodes can not be set to expand if the expansion does not respect constrained FP semantics.


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

https://reviews.llvm.org/D70226