On May 23, 2007, at 8:56 AM, Dan Gohman wrote:
Ok, I now have a patch that implements this. It's a work in
progress, and
still rough in some areas (some details below), but it works well
enough
to allow the LLVM regression tests to pass on x86. I'm posting it
now so
that people can see what I'm up to.
Cool!
On Wed, May 23, 2007 at 12:03:30AM -0700, Chris Lattner wrote:
On Mon, 21 May 2007, Dan Gohman wrote:
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.
Last time I tried this it caused an impressive amount of register
pressure;
long-vector loads/stores require special ScheduleDAG dependencies,
but LLVM
was forcing long-vector dependencies on all the operators so that
operations
on the individual sub-vectors couldn't be scheduled around to
reduce register
pressure. But that's a different topic :-).
In theory -combiner-alias-analysis should fix this, or at least help
it, by making the stores independent of each other so they can be
reordered. It isn't on by default because of its compile time hit.
At some point, we should finish it up.
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??)
urk indeed...
My current patch is specific to vectors, as the table elements for
extended
types are std::pairs of vector lengths and element types. Perhaps
the thing
to do then is to make it a table of Type*. Then it would be usable
for any
machine-illegal type.
Sure, it makes sense to get a start, and extend it from there.
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!
One downside is that debuggers no longer print MVT::ValueTypes with
enum
names.
Slightly annoying, but not a big deal.
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.
I wasn't sure if it should go in the SelectionDAG or the
TargetLowering.
It should be in SelectionDAG. The protocol is that the interfaces in
include/llvm/Target/* are immutable once created (though the
interfaces can hack on data structures passed in, like
SelectionDAGs). The classes instantiated from include/llvm/CodeGen
represent code as it is being hacked on. As such, CodeGen/
SelectionDAG is the right place to go.
My current patch just uses a global table because it was easier and
allowed
me to get to the LegalizeDAG.cpp changes. But I'll definately clean
this up.
Sounds good.
Some quick thoughts about the patch:
diff -u -r1.34 ValueTypes.h
--- include/llvm/CodeGen/ValueTypes.h
+++ include/llvm/CodeGen/ValueTypes.h
Meta-comment about this file: when the table is moved to not be
global, the 'simple' functions should stay, the complex ones should
move to be methods on SelectionDAG. Note that tools like tblgen use
ValueTypes.h (for simple VTs), but don't link the llvm libraries in.
// iPTR - An int value the size of the pointer of the current
// target. This should only be used internal to tblgen!
-iPTR = 255
+iPTR = 255,
Can there be more than 255 elts in the table? If so, I'd suggest
turning this into ~0U or something.
- /// MVT::isInteger - Return true if this is a simple integer, or a
packed
+ typedef int ValueType;
Can these be negative? If not, plz use unsigned instead of int.
+ /// MVT::isExtendedIntegerVector - Test if the given ValueType is a
+ /// vector (simple or extended) with a floating-point element type.
+ bool isExtendedFloatingPointVector(ValueType VT);
comment pasto.
+ /// MVT::isVector - Return true if this is a simple vector value
+
