[LLVMdev] Long-Term ISel Design

[Chris Lattner]

On Mar 16, 2011, at 1:44 PM, David Greene wrote:

> All,
> 
> As I've done more integrating of AVX work upstream and more tuning here,
> I've run across several things which are clunky in the current isel
> design.  A couple examples I can remember offhand:
> 
> 1. We have special target-specific operators for certain shuffles in X86,
>   such as X86unpckl.  I don't completely understand why but Bruno
>   indicated it was to address inefficiecies.  One of those is the need
>   to check masks multiple times (once at legalize and again at isel).

It also eliminates a lot of fragility.  Before doing this, X86 legalize would have to be very careful to specifically form shuffles that it knew isel would turn into (e.g.) unpck operations.  Now instead of forming specific carefully constructed shuffle masks (not making sure other code doesn't violate them) it can just directly form the X86ISD node.

> 2. Sometimes DAGs are legal in some contexts but not others and it is a
>   pain to deal with.  A good example is VBROADCAST, where a <0,0,0,0>
>   shuffle is natively supported if the source vector is in memory.
>   Otherwise it's not legal and manual lowering is required.  In this
>   case the legality check is doing the DAG match by hand, replicating
>   what TableGen-produced code already does.

Yes, this isn't good.  Instead, the shuffle should be legalized to something that takes a pointer (memory operand).  That means that X86 isel would form the *fully legal* X86ISD node, and nothing would be able to break it and it could never fail to match.

> These two examples are related: we're duplicating functionality manually
> that's already available automatically.

Not sure what you mean by this.

> As I've been thinking about this, it strikes me that we could get rid of
> the target-specific operators and a lot of other manual checks if we
> just had another isel phase.  Let's say we structured things this way:
> 
>                  legalize
>                     |
>                     V
>        manual lowering (X86ISelLowering)
>                     |
>                     V
>         manual isel (X86ISelDAGToDAG)
>                     |
>                     V
>    table-driven isel (.td files/X86GenDAGISel)
>                     |
>                     V
>      manual isel (some to-be-design piece)
> 
> The idea is that we keep the existing manual pieces where they are to
> clean things up for TableGen-based isel and/or handle special cases.
> Maybe we consider getting rid of some in the future but that's a
> separate questions.

I'm not sure what you mean here.  Are you suggesting that these be completely separate passes over the dag?  Why do "manual isel" and "table driven isel" as separate passes?  If they are interlaced, then how is this different than what we already have?

> The way things are now, if table-driven isel fails the codegen aborts.
> In the above scheme we get one last chance to do manual lowering before
> we give up.  This helps the shuffle mask case by turning this:
> 
>                             legalize
>                                |
>                                V
>               check shuffle mask legality (X86ISelLowering)
>                                |
>                                V
>       check shuffle mask legality (table-driven isel predicates)

You're saying that we do this on a node-by-node basis?  The reason that codegen aborts on unselectable operations is that they are invalid and should not be formed.  Your example of vbroadcast is a great one: the X86ISD node for it *should not take a vector register input*.  If it does, then the X86ISD node is incorrectly defined.

-Chris