[PATCH] D71127: [ARM] Add custom strict fp conversion lowering when non-strict is custom

[John Brawn via Phabricator via llvm-commits]

john.brawn marked 2 inline comments as done.
john.brawn added a comment.

In D71127#1774339 <https://reviews.llvm.org/D71127#1774339>, @dmgreen wrote:

> Looks sensible.
>
> Is it worth un-xfailing the test, now that it no-longer crashes? Even if it's check the poor codegen (maybe with some FIXME's), we can show it improving over time. I would use that update script, but I'm not sure how verbose that test will be with it.


Un-xfailing it isn't trivial as codegen is poor in an inconsistent manner, e.g. fptosi_f32 gets poor codegen when you have single-precision+double-precision but good codegen when you have single-precision only (despite it having nothing to do with double-precision) due to some weirdness in type legalization.

I could make the test more complicated to handle this, but I'd rather not.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5398
+    if (IsStrict)
+      return DAG.getMergeValues({ Tmp.first, Tmp.second }, Loc);
+    return Tmp.first;
----------------
dmgreen wrote:
> Maybe just `if (IsStrict) return Tmp; else return Tmp.first;` (or something like it), if that works.
Unfortunately not (Tmp is a pair of SDValue), though doing a similar std::tie thing makes it more explicit what's happening so I'll do that.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16222
              "Unexpected type for custom-lowering FP_EXTEND");
-      SrcVal =
-        makeLibCall(DAG, LC, MVT::f32, SrcVal, CallOptions, Loc).first;
+      auto Tmp = makeLibCall(DAG, LC, DstVT, SrcVal, CallOptions, Loc, Chain);
+      SrcVal = Tmp.first;
----------------
dmgreen wrote:
> Can we use std::tie(SrcVal, Chain) = makeLibCall(..) ?
Yes, I'll do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71127