[LLVMdev] DAGCombiner rant

[Dan Gohman]

Hi Scott,

I'm not clear on what you're saying here; some of your
points below seem to be contradictory. The advice to
use target-independent nodes when feasible seems
sound to me, so I wrote up a comment about it in
SelectionDAGNodes.h. If you can formulate your
thoughts in the form of specific documentation changes,
that would be helpful.

In theory, DAGCombiner is supposed to check if an operation
is legal before using it, when it is running after legalize.
It seems in practice it often doesn't do this. Our usual
response when people hit this problem is "then fix it".
Are you saying that the extent of the problem makes this
infeasible?

Dan

On Jan 25, 2009, at 8:04 PM, Scott Michel wrote:

> Yes, it was I who put that rant in the commit log and it's  
> justified. Worse, it's unreasonable to actually go through all of  
> DAGCombiner's code and check to see if certain kinds of constants,  
> e.g., i64, are legal during a particular phase of DAGCombiner.  
> DAGCombiner does good work and the backends are supposed to be good  
> citizens. CellSPU is certainly trying to be a good citizen, no  
> matter how frustrating that becomes on certain days.
>
> We need to document this knowledge somewhere:
>
>  (a) If you're going to legalize operations, you should constrain  
> yourself to using target independent nodes insofar as feasible.  
> Think twice before adding a new pseudo-instruction: it prevents  
> DAGCombiner optimizations and sometimes DAGcombiner will  
> unintentionally work around your operation legalizations in  
> unexpected ways (i.e., transforming (fabs:f64 arg) to  
> (bitconvert:f64 (and:i64 (bitconvert:i64 arg), 0x7fffffffffffffff)))

>
> (b) If you add new pseudo-instructions (i.e., target-specific nodes  
> in your ISelLowering source), make sure you know how to  
> PerformDAGCombine. If there's no possible way to do  
> PerformDAGCombine on your new pseudo-instructions, see (a).
>
> (c) If at all possible, handle target-specific instruction  
> selection, e.g., 64-bit loads or other speciality instruction  
> selection, in your ISelDAGToDAG source. For example, CellSPU cannot  
> lower 64-bit add, substract or multiply operations using target- 
> independent nodes. These are custom instruction-selected using  
> pseudo-instructions that only the instruction selector will see.
>
> (d) If (a), (b) or (c) don't work, then, and only then, create a new  
> target-dependent node and its corresponding ISD-extended pseudo  
> instruction.
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev