[llvm-commits] Speeding up instruction selection (SelectionDAGISel)

[Dan Gohman]

On May 6, 2008, at 5:55 AM, Roman Levenstein wrote:

> Hi Even, Hi Dan,
>
> Here is a patch as discussed in this mail-thread.
>
> 1) It changes the tablegen and moves the common part of all
> instruciton selectors into include/llvm/CodeGen/IDAGISelHeader.h file.
>    Tablegen now only generates the include preprocessor directive for
> this file.
>    I prefer this method to extending the SelectionDAGISel parent
> class, because it may provide more flexibility. For example, based
>    on some #defines controlling the type of the code selector and
> generated by TableGen based on the comman-line parameters
>    different versions of the code selection algorithms cam be used.

Ok.

>
>
> 2) I provide in the include/llvm/CodeGen/IDAGISelHeader.h the current
> standard LLC implementation.
>
> So, the current patch only re-factors the LLVM and tablegen code, but
> introduces no functionality change.
>
> Regarding the speed-up: In my previous report about this patch I did a
> mistake by measuring the times for the DEBUG version of llc.
> There it is really a win (up to 20% on some use-cases, as I reported
> before)! But for the optimized version of the llc, the win is almost
> minimal :-(
> Good lesson for me: never rely on the performance data produced by a
> debug build. They are totally different from release builds in many
> cases.

Ok.

>
>
> 3) I can later provide provide the implementation of of my previous
> patch that does instruction selection
>    without using any sorted data-structures and achieves very good
> speed-ups due to this fact. But the question is:
>    Does it still make sense, if the performance improvement for
> optimized builds is minimal?

I think it still does still have some benefit. It makes the code
simpler, and, I expect it will speed up the EXPENSIVE_CHECKING build
even more than the debug build, due to the way that _GLIBCXX_DEBUG
adds extra-complex code for checking the heap-manipulation functions.

>
> For both standard and my implementation of instruction selection  
> without queues:
>   All Dejagnu pass without any problems.
>   No problems on MultiSource.

Great!

>
>
> Please review and tell, if it is OK to commit this patch.
> And indicate if the approach without queues is still interesting.

At a glance, the patch looks good. Please copy the
"NOTE: This file is #included into the middle ..." comment into  
DAGISel.h
to clarify what this header is for.

The LLVM 2.3 release branch will be created this Friday; please wait
until after the branch is created before committing this.

Thanks,

Dan