[PATCH] D67128: [X86] Move x86_64 fp128 conversion to libcalls from type legalization to DAG legalization

[Chih-Hung Hsieh via Phabricator via llvm-commits]

chh added a comment.

I applied this patch to current Android clang/llvm toolchain to build clang compiler.
Not all toolchain was built due to dependencies on other clang/llvm changes not picked into Android yet.
We will known only later when Android toolchain is updated with this and all dependent changes
whether f128 type still works on Android x86_64 devices.
As long as we keep this and future dependent changes small, we should be able to manage any regression found later.

So far new clang compiler built with this change can compile all Android code, especially the libm.
I didn't run on device test due to other missing components. The output code of libm look good.
Some .o files have increased size but overall libm.a and libm.so size difference is negligible, less than 0.1%.

I added some trivial inline comments.
I think this change is good to submit after those issues are addressed.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:687
+    setOperationAction(ISD::FP_EXTEND, MVT::f128, Custom);
+    // We need to custom handle any FP_ROUND with an ff128 input, but
+    // LegalizeDAG uses the result type to know when to run a custom handler.
----------------
s/ff128/f128/



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18694
 
-  if (Op.getSimpleValueType().isVector())
+  if (SrcVT.isVector())
     return lowerUINT_TO_FP_vec(Op, DAG, Subtarget);
----------------
Is the change to SrcVT at line 18694 to fix some other bug or an error?
Before this change, it was Op.getSimpleValueType() or DstVT.
Why is it changed to SrcVT?




================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19520
+  if (VT == MVT::f128) {
+    RTLIB::Libcall LC; LC = RTLIB::getFPEXT(SVT, VT);
+    return LowerF128Call(Op, DAG, LC);
----------------
Coding style comment.
Shouldn't it be
  RTLIB::Libcall LC = RTLIB::getFPEXT(SVT, VT);



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67128