[PATCH] D11438: Fix x86_64 fp128 calling convention

[Chih-Hung Hsieh via llvm-commits]

chh added a comment.

David, I have reconsidered a few alternatives and checked again current llvm architecture.

First a few observations for other people not following all the issues before:

  The core problem is that llvm classifies types into
  legally-in-register-with-HW-instruction or
  illegal-and-should-be-expanded-promoted-or-soften.
  f128 is classified as illegal for most targets and soften to i128,
  which is passed/returned in two 64-bit registers.
  The SoftenFloat* functions not only convert operations to library function calls,
  but also the result type to i128. 
  But on x86_64, calling convention for i128 is different from f128.
  GCC and llvm are correct for i128, but llvm incorrectly passed f128 the same as i128.
  
  To give f128 its own calling convention, I still don't have other solution but
  to create its own register class. That makes it a new kind of type not well fit
  into current scheme.
  
  It does not seem feasible to me to keep using SoftenFloat* functions and
  convert f128 to i128, and then add other hacks to use different calling convention
  for real i128 and f128-converted-i128. If it is possible, please let me know
  how to do it.

If I understand correctly, your suggestion is to fix some SoftenFloatRes_* functions
to return f128 for f128 in this case instead of skipping them.
If we want to fix them and avoid the changes to LegalizeTypes.cpp, they will
also need to scan-and-convert operands, because current LegalizeTypes.cpp simply
"goto NodeDone" after calling SoftenFloatResult.
Wouldn't that change duplicate the ScanOperands actions from LegalizeTypes.cpp to
LegalizeFloatTypes.cpp?

I did not like the duplication of ScanOperands, so I added new return value to
SoftenFloatResult to indicate if ScanOperands is necessary. Then, some
node types no longer need any change inside SoftenFloatResult.

Maybe what I should do is adding more comments to SoftenFloat* functions to
explain that they now do not always convert floating point types to integer types.
In fact, they convert nodes to the TransformToType[VT], which could be f128 or i128.
Current comment of getTypeToTransformTo says:

  /// For types supported by the target, this is an identity function....

That can be extended to as "supported by the target or passed/returned in registers".

Another alternative is to keep the exact meaning of LegalizeTypeAction::TypeSoftenFloat
and add a new action like TypeSoftenOnlyFloatOps.  We still need to change comments of
several functions to deal with the new legally in register but no HW support f128 type.
For other targets, with the same calling convention for f128 and i128,
they can keep on using TypeSoftenFloat, which still converts nodes to the
TransformToType[VT] but that is assumed to be an integer type.
For x86_64, the new TypeSoftenOnlyFloatOps action will assume that TransformToType[VT]
could be the same as VT and VT could be legally in register.

At the end, SoftenFloat* and SoftenOnlyFloatOps* functions could share some code
to minimize duplication. The shared code would have some flag to serve similar purpose
as the KeepFloat variable in my patch.
Many places that handle LegalizeTypeAction::TypeSoftenFloat will need to be changed
to also handle LegalizeTypeAction::TypeSoftenOnlyFloatOps.
That doesn't seem to be a simpler patch either.

BTW, I am going to LLVM dev meeting too.
So if I don't have time left this week, I will continue next week.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:726
@@ +725,3 @@
+  EVT OpVT = N->getOperand(OpNo).getValueType();
+  bool KeepFloat = OpVT.isSimple() && TLI.isTypeLegal(OpVT);
+
----------------
davidxl wrote:
> Why is keepFloat computed differently from SoftenFloatResult?
SoftenFloatResult converts its given SDNode N,
so its KeepFloat is computed from N's value type:

  EVT VT = N->getValueType(ResNo);
  EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), VT);
  bool KeepFloat = VT.isSimple() && VT == NVT && TLI.isTypeLegal(VT);

SoftenFloatOperand converts N's operand,
so its KeepFloat is computed from the OpNo operand:

  EVT OpVT = N->getOperand(OpNo).getValueType();
  bool KeepFloat = OpVT.isSimple() && TLI.isTypeLegal(OpVT);

The extra check of VT == TLI.getTypeToTransformTo(....)
is not really necessary when we have only x86_64 f128 that
can be legal and transformed to itself at the same time.
I can remove it, or keep it as an redundant check in both places
to filter out any other new unexpected configuration.

Is this the difference you noticed?
Would you rather keep or remove (VT == NVT ) at both places?


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:381
@@ -378,1 +380,3 @@
     SDValue &SoftenedOp = SoftenedFloats[Op];
+    if (!SoftenedOp.getNode() &&
+        Op.getValueType().isSimple() &&
----------------
davidxl wrote:
> If SetSoftenedFloat was called with the right value, then there is no need to do special handling here. This is the reason why I think SoftenFloatResult should not special handling those op code.
In LegalizeTypesGeneric.cpp, you can see that a caller of GetSoftenedFloat
assumes that a softened float should be split into two integers.
We don't want that. So either we modify the meaning of TargetLowering::TypeSoftenFloat
and GetSoftenedFloat or we need to use a new action type like TypeSoftenOnlyFloatOps.



http://reviews.llvm.org/D11438