[legalizetypes] Push fp16 -> fp32 extension node to worklist.

[fhahn]

Summary

This way, the type legalization machinery will take care of registering
the result of this node properly.

This patches fixes all failing fp16 test cases with expensive checks.
(CodeGen/ARM/fp16-promote.ll, CodeGen/ARM/fp16.ll, CodeGen/X86/cvt16.ll
CodeGen/X86/soft-fp.ll)

[fhahn]

Ping. Any thoughts on the patch?

[RKSimon]

The fp128 failures on PR30999 are failing from something similar

[fhahn]

@RKSimon Yes I think some of the remaining XDEBUG failures for X86 are due to related issues. I'll try to have a look soon.

[fhahn]

I just checked, this patch fixes the following failing tests with expensive checks:

LLVM :: CodeGen/ARM/fp16-promote.ll
LLVM :: CodeGen/ARM/fp16.ll
LLVM :: CodeGen/Thumb2/float-intrinsics-double.ll
LLVM :: CodeGen/X86/cvt16.ll

[fhahn]

Seems like there haven't been many changes to LegalizeTypes recently. I'm adding more reviewers based on some of the last reviews in the area (from 2015 and 2016), I would appreciate any comments.

[hfinkel]

lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp

462

To some extent, this makes sense. The caller of SoftenFloatResult, DAGTypeLegalizer::run, is also what sets the node ID to Processed.

I wonder if there's a better design here, because I share your uncertainty about all of the relevant legalities being satisfied. It seems like what we're doing is taking

FP_EXTEND f16 x -> fn

and rewriting them as:

FP_EXTEND (FP_EXTEND f16 x -> f32) -> fn

where we're trying to directly recurse on that inner operation. I don't understand why we're trying to do that as opposed to just queueing the new node for processing. Would it work to just do:

Worklist.push_back(Op.getNode());

instead of calling SoftenFloatResult?

[fhahn]

lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp

462

Thank you very much for having a look! It's indeed possible to push the node to the worklist and let the legalization machinery take care of it.

I had to remove the call

Op = GetSoftenedFloat(Op);

because now that SoftenFloatResult isn't called anymore, GetSoftenedFloat will fail (I think it should only be called in the cases we called SoftenFloatResult previously.

[hfinkel]

lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp

462

Great, thanks!

Given that we also need to call setNodeId to make this work, can/should we add an addToWorklist member function that encapsulates the Worklist.push_back + setNodeId?

[fhahn]

Add AddToWorklist helper function.

[hfinkel]

LGTM

lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp

SDValue DAGTypeLegalizer::SoftenFloatRes_FP_EXTEND(SDNode *N) {
   EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), N->getValueType(0));
   SDValue Op = N->getOperand(0);
 
   // There's only a libcall for f16 -> f32, so proceed in two stages. Also, it's
   // entirely possible for both f16 and f32 to be legal, so use the fully
   // hard-float FP_EXTEND rather than FP16_TO_FP.
   if (Op.getValueType() == MVT::f16 && N->getValueType(0) != MVT::f32) {
     Op = DAG.getNode(ISD::FP_EXTEND, SDLoc(N), MVT::f32, Op);
-    if (getTypeAction(MVT::f32) == TargetLowering::TypeSoftenFloat)
-      SoftenFloatResult(Op.getNode(), 0);
+    if (getTypeAction(MVT::f32) == TargetLowering::TypeSoftenFloat) {
+      Worklist.push_back(Op.getNode());
+      Op.getNode()->setNodeId(ReadyToProcess);
+    }
   }
 
   if (getTypeAction(Op.getValueType()) == TargetLowering::TypePromoteFloat) {
     Op = GetPromotedFloat(Op);
     // If the promotion did the FP_EXTEND to the destination type for us,
     // there's nothing left to do here.
     if (Op.getValueType() == N->getValueType(0)) {
       return BitConvertToInteger(Op);
     }
   }
 
   RTLIB::Libcall LC = RTLIB::getFPEXT(Op.getValueType(), N->getValueType(0));
-  if (getTypeAction(Op.getValueType()) == TargetLowering::TypeSoftenFloat)
-    Op = GetSoftenedFloat(Op);
   assert(LC != RTLIB::UNKNOWN_LIBCALL && "Unsupported FP_EXTEND!");
   return TLI.makeLibCall(DAG, LC, NVT, Op, false, SDLoc(N)).first;
 }
 
 // FIXME: Should we just use 'normal' FP_EXTEND / FP_TRUNC instead of special
 // nodes?
 SDValue DAGTypeLegalizer::SoftenFloatRes_FP16_TO_FP(SDNode *N) {
   EVT MidVT = TLI.getTypeToTransformTo(*DAG.getContext(), MVT::f32);