[PATCH 1/1] R600: Use TargetConstant in LowerConstantInitializer

[Matt Arsenault]

On May 21, 2014, at 9:30 AM, Matt Arsenault <arsenm2 at gmail.com> wrote:

> 
> On May 21, 2014, at 2:28 AM, Matt Arsenault <arsenm2 at gmail.com> wrote:
> 
>> 
>> 
>> So I think this is sort of correct, but I don’t know why this function is doing what it’s doing. I’m pretty sure you don’t want to use TargetConstant to fix this. It’s copying the constant initializer onto the “stack”, but expanding it into a legal type so the private memory copy doesn’t have the exact same layout as in the constant. I don’t know if that’s OK or not since I don’t know why this is copying it.
>> 
>> Besides that, I think it would be better to get the type to use with TLI.getRegisterType() rather than using getSmallestLegalIntType. You should also keep the variable as MVT since you’re trying to only use legal types here.
>> 
> 
> I think the copying is a workaround for not supporting really setting up the constant buffer. The constant initializer should really be copied into some data section of the binary, and the runtime should load the data into a buffer the kernel will read from, but that seems to not be implemented.

Which would also make this incorrect, since what you really want is a truncating store to the correct size at the correct address, but this will store a second i8 in an array 4 bytes later which is wrong. I’m not sure how much effort there is in fixing this workaround vs. really loading the constant data.

I noticed a few other problems with this, such as structs and vectors being broken (which I’ve similarly partially fixed), which also are made more difficult by this coming after vector legalization. These issues would just go away if if the global address was replaced with an implicit kernel argument that the runtime loads the data into.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140521/70bda1c5/attachment.html>