[llvm-commits] [patch] Add a pass for duplicating indirectbr at the IL level

[Bob Wilson]

I've also been working on this problem.  I've resurrected the work I had started last year to have a smarter strategy for duplicating indirect branches.  I don't have anything ready to commit yet, but it's coming along (slowly).  I'm trying to solve a broader set of problems than you are tackling with this patch.  For example, to pick just one, your new pass can still only duplicate past unconditional branches.  That's not good enough for something like JavaScriptCore.  You also have the same problems as the existing pass with excessive duplication.  But, since it might take me a while to finish my work, I won't have any objections to taking something like this patch if it doesn't conflict with where I think we should be heading in the longer term.

It looks to me like you've basically taken the existing MI-level pass, moved it to the IL-level and added a special-case check for whether the indirect branch address is coming from a load.  What if it's not a load?  I'm assuming that you're doing this to expose the load in the selection DAG, right?  An alternative might be a target-specific peephole optimization to merge the load with the indirect branch at the MI level?

One big concern I have is that this patch would be forcing us to have two separate implementations of the SSA updates, one here at the IL level and another in the existing MI level early tail dup pass.  As you should remember, I was pretty hesitant to go along with your recent changes to the early tail dup pass, because I was hoping to remove it entirely, thereby removing all the SSA complexity from tail duplication.  But, since you showed that there were some significant benefits, I agreed that it was the right thing to do, with the hope that we could share the SSA updating code between early tail dup and the smarter indirect branch dup pass that I'm working on.  I suppose we might be able to templatize that SSA updating code in the same way that the SSAUpdater is templatized to work at both levels, but that's a big hassle all around and I'd really like to avoid it.

On Jul 19, 2011, at 8:56 PM, Rafael Ávila de Espíndola wrote:

> The attached patch adds a pass for duplicating indirectbr at the IL
> level. The motivation is to get better instruction selection. For
> example, with the current MI based duplication, we get
> 
> leaq    table(,%r11,8), %r8
> jmpq    *(%r8)
> 
> with this patch we get
> 
> jmpq    *table(,%r11,8)
> 
> 
> The pass is mostly the current CodeGen TailDuplication with ideas taken
> from the old IL TailDuplication (it looks like this one is unused,
> should it be deleted?).
> 
> With this patch in we can probably remove the bias for duplicating
> indirectbr in CodeGen.
> 
> The two main points I would like a review are the two fixmes in the
> code: What it is the best place to factor the "isDefLiveOut" function
> and if SSAUpdate should be extended to handle the case mentioned by the
> other FIXME.
> 
> Thanks,
> Rafael
> <indirectbr.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits