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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 15:33:53 PST 2017

fhahn added inline comments.

Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:463
       SoftenFloatResult(Op.getNode(), 0);
+      Op.getNode()->setNodeId(Processed);
+    }
hfinkel wrote:
> 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?
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.


More information about the llvm-commits mailing list