[LLVMdev] visitBIT_CONVERT (previous Shouldn't DAGCombine insert legal nodes?)

[Duncan Sands]

Hi Gabrielle,

> > Historically nodes marked "custom" were considered legal, so the
> > DAGCombiner would have been correct to generate it.  Not sure how
> > that ever worked though.  I think Dan split the isOperationLegal
> > method into isOperationLegal and isOperationLegalOrCustom for reasons
> > related to this kind of thing.  I don't know whether the DAGCombiner
> > is now only supposed to produce legal non-custom nodes.
> In my understanding, "Custom" means that I'm in charge to build the node. So, "Custom" means "Legal" once the node has been build by my code (through my implementation of the TargetLowering::LowerOperation() method).
> The DAGCombiner could call TargetLowering::LowerOperation() when it needs a "Custom" node, but it doesn't. So, it knows that the node needs a custom build but it doesn't ask anything to the guy (the code) in charge of building custom nodes.
> The combiner is replacing some nodes by some other nodes because it thinks this is better. So, anyway, it is preferable to not use custom nodes because these nodes can result in a higher number of nodes. In my case, the "Custom" i64 AND node should be replaced by 2 i32 AND modes and few glue nodes, making the final DAG more complex than before the combine step. This makes no sense.
> I plan to solve this problem by doing the reverse operation the combiner did. During selection (in SelectionDAGISel::Select()), I check for the node pattern having a i64 AND between an i64 constant 0x7FFFFFFFFFFFFFFF and a bit_convert, and I replace this by a fabs and a bit_convert node. In this way, this portion of the DAG will look as before the "combine 2" step. I think this is not so far from what Michel suggested.

how about this instead: modify the DAGCombiner so that before operations
have been legalized, the DAGCombiner is allowed to produce nodes satisfying
isOperationLegalOrCustom, while after operations have been legalized it is
only allowed to produce nodes satisfying isOperationLegal.

> Have I to make a bug report ?

Sounds like a good idea.

> Is there any predictable order on which all nodes are going through SelectionDAGISel::Select().

I don't know.

Ciao,

Duncan.