<div class="gmail_quote">On Wed, Jan 28, 2009 at 1:42 PM, Dan Gohman <span dir="ltr"><<a href="mailto:gohman@apple.com">gohman@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi Scott,<br>
<br>
I'm not clear on what you're saying here; some of your<br>
points below seem to be contradictory. The advice to<br>
use target-independent nodes when feasible seems<br>
sound to me, so I wrote up a comment about it in<br>
SelectionDAGNodes.h. If you can formulate your<br>
thoughts in the form of specific documentation changes,<br>
that would be helpful.</blockquote><div><br>Sorry if I wasn't clear, since I was venting some of my frustration with my continual battle with DAGCombine and the CellSPU backend. DAGCombine should operate according to the principle of least surprise; yet, I'm continually being surprised.<br>
<br>My biggest frustration, until recently, was finding new i64 constants being generated by DAGCombine after operation legalization. Prior to that, InstCombine was inserting i64 multiplications; I ended up biting the bullet and generating the full 48+ instruction sequence for i64 mul for CellSPU in terms of the i16 mulipliers that Cell's SPU actually support.I moved all of i64 constant generation to instruction selection, when originally, i64 constants were legalized by target-specific nodes.<br>
<br>My approach now is the path of least pain and better citizenship within the LLVM framework itself.<br><br>Specfic guidance for backend developers:<br><br>(a) Avoid target-specific nodes.<br>(b) Legalize operations in terms of existing ISD instructions.<br>
(c) Don't be surprised if DAGCombiner is too smart for your backend's capabilities.<br>(d) The size and scope of changing DAGCombiner is daunting, so if you can avoid changing or altering DAGCombiner, don't change DAGCombiner. For example, how many changes need to be made to check of i64 constants are legal during a particular phase of DAGCombiner -- you have to do a __complete__ job, not just patch the place that just bit you in the backside.<br>
(e) Handle as much target-specific <foo> during instruction selection (e.g., i64 constants). IFF there's no way to handle <foo> during instruction selection, then consider adding a new target-specific node for operation legalization.<br>
(f) If in doubt about any of the above suggestions, refer to (e) and (a).<br><br>Regarding (d), I'd rather do a complete job than a half-baked job if I were going to submit a patch against DAGCombiner. Looking at the code, even using NetBeans' IDE, tracking down "illegal" i64 constants is a daunting task. The path of least pain was to move i64 constant generation to instruction selection and leave DAGCombiner alone. Basically, the guidance is "Do as much of what you can during instruction selection, rewrite operations as target-independent nodes insofar as feasible, and only as a last resort, add new target-specific nodes because the pain threshold for altering DAGCombiner or SelectionDAGLegalize is sufficiently intolerable." (Yes, all one needs to look for are occurances of "DAG->getConstant(MVT::i64", but there's the possibility of either getting it wrong or being incomplete.)<br>
<br>In (e), however, most backed developers will immediately notice that they have to run a set of predicates that will be executed a second time by the generated Select() code. There's probably a better way to do that.<br>
<br>Example: Cell's SPU is a vector processor with a "unform register set", meaning that scalars are always in slot 0 of the vector (mostly.) Unformity has to be preserved at all times. Hence, at least in CellSPU's case, i64 constants __have__ to be represented as BUILD_VECTOR nodes and the various selection predicates are run against the node to determine if the result is a simple constant load that propagates to all vector elements or whether a constant pool spill needs to occur (depends on the constant and avoiding a CP spill.) If a predicate succeeds, then the generated Select() method is invoked, which runs all of the predicates a second time.<br>
<br>Hope that clears up some of the confusion.<br><br><br>-scooter<br></div></div><br>