[llvm-commits] DAGCombiner Patch: Allow targets to do combinefirst.

[Sanjiv.Gupta at microchip.com]

> 
> > The DAGCombiner::combine() in its existing form does not allow
> > targets to do anything if the visitNODE() has done something with
it.
> > IMO, the targets should always be allowed to do stuff irrespective
> > of what we want to do in visitNODE().
> 
> DAGCombine is iterative; I believe the target hook will be called
> after DAGCombine is done hacking with it. I guess what your patch does
> is add a hook to let the target intervene before DAGCombine runs. One
> of the main purposes of the first DAGCombine runs is to clean up and
> canonicalize the mess that SelectionDAGLowering emits so that the rest
> of CodeGen doesn't have to deal with it. Consequently, it's not clear
> why it would be useful to give targets a hook to step into the mess,
> in general.
Sometimes a target knows better that a few things in this mess are
really unwanted for it; so it can pitch in early and take care of the
cleaning process rather than just wait for the combine to make its job
more difficult. To be specific, in our case the caller directly saves
the arguments on callee's stack and hence those extra stores generated
by clang to store the argvals to callee's frame indices are unwanted. I
mentioned the problem in case you might get some better ideas to solve
the problem. 

If you didn't like this idea of moving the call to an early place, then
we can probably think of another hook called PerfromDAGPreCombine() that
pitches in earlier than visitNODE().

> And in the specific case that's motivating this, it seems
> that trying to fix this problem by selectively disabling optimizations
> seems to be swimming upstream.
> 
I agree that this is probably a better way to solve the problem.
That functionality will anyway be required because there might be few
other cases in which one may need to do that. I will probably keep a
bitmask that a TLI hook can use to set the switches ON/OFF.

> > Attached is a patch for same. Let me know if it sounds ok. I will go
> > ahead and commit it.
> 
> Please send normal attachments. winmail.dat seems to be some
> Outlook-specific thing.
> 

I didn't how this .dat thing got into it. I was on Linux when I attached
a .txt patch. 

> LLVM's CodeGen is designed for register architectures. When it
> is used to target stack architectures, the preferred approach
> seems to be to do instruction selection as if the target had
> registers and instructions that operate on registers, and then
> to lower everything to stack operations in a later phase. 

We do not even maintain a run-time stack. The current model (statically
assigned stack) works well for us where we have defined only one
register. The regalloc knows it and generates spilling/reloading
accordingly.

Did you mean that we have a separate regclass posing mem bytes as regs
and lower things after regalloc is done? We do not even have membyte to
membyte copy insn. Well, TBH, I did not quite understand it. 

- Sanjiv