[llvm-commits] [llvm] r91614 - in /llvm/trunk: include/llvm/CodeGen/ValueTypes.h lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

[Duncan Sands]

Hi Ken,

> Introduce EVT::getHalfSizedIntegerVT() for use in ExpandUnalignedStore() in
> LegalizeDAG.cpp. Unlike the code it replaces, which simply decrements the simple
> type by one, getHalfSizedIntegerVT() searches for the smallest simple integer
> type that is at least half the size of the type it is called on. This approach
> has the advantage that it will continue working if a new value type (such as
> i24) is added to MVT.

I don't think this is correct.  Consider an unaligned store of i24.  This should
write 3 bytes to memory.  It looks like getHalfSizedIntegerVT will return i16.
ExpandUnalignedStore will then write an i16 followed by another one, writing
4 bytes, which is wrong.  What should happen is: if the alignment is 1, then
three lots of i8 should be written; if the alignment is 2 then an i16 followed
by an i8 should be written.  How about rewriting ExpandUnalignedStore so that
it works as follows.  First it finds the largest legal integer type T with the
given alignment (eg: i8 for alignment 1, i16 for alignment 2) that is smaller
than the original type.  It then does the maximum possible number of T stores.
If the entire value wasn't stored then replace the original type with the left
over bits and repeat.  For example, suppose i8 has alignment 1, i16 alignment 2,
i24 alignment 2, i32 alignment 4 and i48 alignment 4, and that all these types
are legal.  Then a store of i48 with alignment 2 would be expanded into two
i24 stores, a store of i64 with alignment 2 would be expanded into two i32
stores, and a store of i56 would be expanded into an i32 store followed by an
i24 store.  Another possibility is to do it recursively, but why bother when
you can do a loop!

Another flaw is that getHalfSizedIntegerVT may return illegal simple types
when you would really rather use legal ones.  For example, suppose i16, i32
and i48 are legal, but i24 is not.  Suppose you do an unaligned store of i48.
Then it would be best to expand this into a store of i32 followed by a store
of i16, rather than two stores of i24.

Ciao,

Duncan.