[PATCH] Fix illegal DAG produced by SelectionDAG::getConstant() for v2i64 type
Daniel Sanders
daniel.sanders at imgtec.com
Fri Oct 18 03:42:00 PDT 2013
> Do we really want to do this for all constants before legalize
> types? It looks like it adds an unnecessary layer of obfuscation at
> this level (optimizer has to look through bitcasts, etc). It also
> wouldn't surprise me if the DAGCombiner just folds it back into an
> illegal constant. Not a big problem, but creates unnecessary churn.
I haven't found an existing way to tell if SelectionDAG::getConstant() is called before the legalizers from within SelectionDAG::getConstant() itself. I've found relevant API's in DAGCombinerInfo (isBeforeLegalize, isBeforeLegalizeOps, isAfterLegalizeVectorOps) but SelectionDAG doesn't seem to keep track of this information.
I can think of two options at the moment:
* The first is to add an optional 'bool IsBeforeLegalizeTypes = false' argument to SelectionDAG::getConstant() and
have SelectionDAGBuilder pass true for each of it's calls. I can then use this argument to enable/disable the code
that ensures legal types are produced.
* The second is to add a 'bool IsBeforeLegalizeTypes' to SelectionDAG and have it's constructor set it to true.
SelectionDAGBuilder would set it to false once it has finished (or the TypeLegalizer could set it when it starts).
The second option seems like the better choice since it can potentially be used for other cases where the DAGCombiner produces something illegal. However, I think SelectionDAG not having the information I'm proposing to add is a deliberate design decision. That said, a simple rename of the variable might remove this concern (perhaps IsBeforeLegalizeTypes should be named NewNodesMustHaveLegalTypes?).
What do you think?
http://llvm-reviews.chandlerc.com/D1973
More information about the llvm-commits
mailing list