[LLVMdev] X86 LowerVECTOR_SHUFFLE Question

[Bruno Cardoso Lopes]

> Maybe.  We still have the old operators like unpck and shup, so couldn't
> those still trigger?  Shouldn't we remove them if we're using this
> TargetNode method?
>
> Is it very expensive to check masks, in the grand scheme of things?

Probably not, in the old scheme the masks could be checked more than
once during legalization and also more than once in the tablegen file
(since the same mask is used in several patterns). But even if it's
not a big improvement in efficiency IMO the "more readable" approach
is still a big win.

>> Although we still match the node itself through tblgen patterns, the
>> tablegen patterns are a lot more clear now, and we were able to remove
>> lots of confusing code.
>
> That's more to do with the way the patterns grew organically than where
> the matching happens.

Yes, but we have to start somewhere...

>> The code used to be *very* fragile, during legalization there was no
>> explicit rule or comments of what and when stuff needed to be matched
>> (and changing a single rule would generate a code that would fail to
>> match tblgen patterns later),
>
> Can you give an example?  When I run into something like that, it's
> almost always because a useful pattern is missing.

I don't remember off hand, but I keep the memories of often running
into different issues because of how fragile the code used to be.

>> all the logic was getting really hard to understand.
>
> Oh yes, it's messy stuff all right.  But as I'm moving patches from 2.7
> to trunk, I'm not noticing much improvement in the legalization code.
> Probably it's still a work in progress, yes?

Yes.

>> The long term plan is: generate all target specific nodes during
>> legalization, and once all logic is clear, we can go for more fancy
>> stuff like having per-processor family shuffle tables describing the
>> more profitable shuffles for a specific mask, and so on. The work
>> stopped because of this bug:
>> http://llvm.org/bugs/show_bug.cgi?id=8156,
>
> Hmm...Again, isn't this due to the matching code being moved from isel
> to legalize?  It seems like legalize is trying to do too much.  It's not
> just "legalize" anymore, it's "legalize and optimize."
>
> When I added support for AVX shuffles (which I am upstreaming right
> now), I tried to do it in a systematic way.  Maybe it will help to
> reorganize the other checks in a similar fashion.  I haven't really
> thought about the bigger picture enough yet.
>
>> but IMHO the implementation of x86 shuffle matching is a lot more
>> clear now then they used to be in the past.
>
> There's certainly been improvement on the TableGen side of things.  I
> really liked the unpck*, shufp, etc. nodes and the ShuffleVectorSDNode.
> That's a huge help.  It's too bad we're getting rid of them.  But
> legalization still looks about the same to me.

The idea is to use tablegen again once we have a clean implementation.
It would be good to have all tables and per-processor logic in a
tablegen file, but I think a clean implementation needs to be done
first. Anyway, I don't have time to work in any of this at the moment,
and I'm not also the one with final word on this. I'm happy to help
with
the bugs that may appear and with some design ideas though.

> Thanks for the explanation.

You're welcome.

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc