[PATCH] D86078: [AArch64] Improved lowering for saturating float to int.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

Sorry I had hoped to look at this patch properly today but that hasn't worked out.  I've got a general comment below as I think it would be nicer to abstract the NativeSaturation into a TLI hook that other targets can implement when useful to them.

I'm also wondering if there's some code reuse opportunities with LowerVectorFP_TO_INT since understandably they're making similar lowering choices. I'll defer that last point until I take a proper look tomorrow.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:3009
+    // will already be mapped to zero.
+    return expandFP_TO_INT_SAT(Op.getNode(), DAG, true);
+  }
----------------
This seems a bit unnatural to the way lowering is typically done.  Can this just return SDValue() and then add a target TLI hook to query the behaviour of the FP convert ISD nodes that expandFP_TO_INT_SAT can call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86078