Simplifing the handling of pre-legalize vector nodes

Right now there are special SelectionDAG node kinds for operations on
"abstract" vector types (VLOAD, VADD, and all the rest), and a special
MVT::Vector ValueType for them. These nodes carry two additional operands,
constants which specify the vector length and element type. All of this is
only used before legalize; then they are replaced with regular node kinds
and value types.

It seems that a number of things would be considerably simpler if the
pre-legalize nodes could use the same node kinds as post-legalize; the only
thing preventing that is that the MVT::ValueType enum isn't able to describe
vector types with arbitrarily long vector lengths.

I'm currently considering ways to make this possible. One idea is to rename
the MVT::ValueType enum to something else and make MVT::ValueType a plain
integer type. Values beyond the range of the original enum are interpreted
as indices into a UniqueVector which holds pairs of vector lengths and
element types. This would require most of the ValueType utility routines
to be aware of the new kinds of ValueTypes so that they could handle them
properly, but once that's done, it would allow vector nodes to be handled
consistently between pre-legalize and post-legalize, without obscure
constructs like
  ... blahblah->getNumOperands()-2 ...
  ... *(blahblah->op_end()-1) ...
to get the vector length and/or element type in the pre-legalize form.

It would make -view-legalize-dags and earlier DAG drawings a lot easier to
read in DAGS with vector nodes, because they wouldn't need to be edges from
each vector node up to the common element type node and vector length node.

Finally, and of particular interest for me, it may help with the next steps
beyond PR400, as it would allow vector loads to use plain LoadSDNode nodes
instead of VLOAD, and thus have an alignment, not to mention a volatile flag
and alias information, in a consistent manner with regular loads.

Before I do much more experimentation with this, I wanted to run the idea by
the list to see what feedback it might get. Has anyone thought about doing
this before? Are there other approaches that might be better?

Another idea is to get rid of the ValueType concept altogether and just use
Type* for everything. I don't know how this would work with various
tablegen'd pieces of code though.

Dan

Sorry for the delay, things are more insane than usual, with the pending release, the dev mtg, etc :slight_smile:

Right now there are special SelectionDAG node kinds for operations on
"abstract" vector types (VLOAD, VADD, and all the rest), and a special
MVT::Vector ValueType for them. These nodes carry two additional operands,
constants which specify the vector length and element type. All of this is
only used before legalize; then they are replaced with regular node kinds
and value types.

Right.

It seems that a number of things would be considerably simpler if the
pre-legalize nodes could use the same node kinds as post-legalize; the only
thing preventing that is that the MVT::ValueType enum isn't able to describe
vector types with arbitrarily long vector lengths.

Yep, I'm sure you know this, but this is to support generic vectors. For example, if you had an input .ll that operated on 128-wide vectors, we want to be able to split that up to use 4-wide vectors if your target supports them.

Going forward, we will also likely have to do something similar to this for integer types, in order to handle stuff like i47 that gets legalized into 2 x i32 or something. I am not looking forward to duplicating all of the arithmetic nodes for IADD variants etc (urk, imagine vectors of strange-sized-integers! VIADD??)

I'm currently considering ways to make this possible. One idea is to rename
the MVT::ValueType enum to something else and make MVT::ValueType a plain
integer type. Values beyond the range of the original enum are interpreted
as indices into a UniqueVector which holds pairs of vector lengths and
element types.

That is a very interesting idea. It handles the generality of arbitrary value types, but is nice and fast for the common code that doesn't use the craziness :). I like it!

This would require most of the ValueType utility routines
to be aware of the new kinds of ValueTypes so that they could handle them
properly, but once that's done, it would allow vector nodes to be handled
consistently between pre-legalize and post-legalize, without obscure
constructs like
... blahblah->getNumOperands()-2 ...
... *(blahblah->op_end()-1) ...
to get the vector length and/or element type in the pre-legalize form.

That would be amazingly wonderful. The other nice thing about this is that 'normal' code that uses (e.g.) sse intrinsics wouldn't have to have all the nodes rewritten multiple times from VADD->ADD->... Anything in a legal type would just be created and stay legal.

It would make -view-legalize-dags and earlier DAG drawings a lot easier to
read in DAGS with vector nodes, because they wouldn't need to be edges from
each vector node up to the common element type node and vector length node.

Also true.

Finally, and of particular interest for me, it may help with the next steps
beyond PR400, as it would allow vector loads to use plain LoadSDNode nodes
instead of VLOAD, and thus have an alignment, not to mention a volatile flag
and alias information, in a consistent manner with regular loads.

Yep. For this specific one, it would be possible to enhance vload, but I agree that this is not a very appealing solution :slight_smile:

Before I do much more experimentation with this, I wanted to run the idea by
the list to see what feedback it might get. Has anyone thought about doing
this before? Are there other approaches that might be better?

This approach sounds very sensible. Make sure the SelectionDAG owns the table though.

Another idea is to get rid of the ValueType concept altogether and just use
Type* for everything. I don't know how this would work with various
tablegen'd pieces of code though.

I'd suggest sticking with ValueTypes. Among other things, they can be indexed into arrays and Type*'s can't. This is important for the various tables in targetlowering that the legalizer consults to find out if a type/operation is valid.

-Chris