[llvm-commits] Struct-handling patches

Duncan Sands baldrick at free.fr
Wed Oct 1 06:04:56 PDT 2008


Hi,

> But where exactly is HasPadding used? Or do you mean "should/could take care
> of this" ?

if HasPadding is not being used, but should, that's a bug.

> In particular, I looked a bit closer at the SimplifyMemTransfer. The only
> thing they do there regarding sizes of the types juggled, is comparing if the
> store size of the struct type that's memcpy'd is equal to the memcpy size.

I suspect the problem is that structs as first class types didn't exist
when SROA was written, and it's basically an accident that it seems to
work for structs.  I doubt anyone carefully audited it to check that the
logic is fine for structs and arrays.  Using the store size was fine as
long as it was only dealing with good 'ol integers and floats.

> Since the store size includes padding, this is probably the case. However, for
> the {double} case ( a double is 10 bytes, right?), this will probably mean
> that a store of type double will not include this padding, while the original
> memcpy didn't.

Exactly.

> Ie, I have this feeling that there is a bug in SimplifyMemTransfer regarding
> this, but I don't have the time to find a testcase right now.

It's surely a bug.  If you have time, can you please review SROA for correctness
in the presence of structs and arrays as first class types.

Thanks,

Duncan.



More information about the llvm-commits mailing list