<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 21, 2014 at 12:57 PM, Matt Arsenault <span dir="ltr"><<a href="mailto:arsenm2@gmail.com" target="_blank">arsenm2@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class="h5"><br>
<div><div>On May 21, 2014, at 9:43 AM, Matt Arsenault <<a href="mailto:arsenm2@gmail.com" target="_blank">arsenm2@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div><br>On May 21, 2014, at 9:30 AM, Matt Arsenault <<a href="mailto:arsenm2@gmail.com" target="_blank">arsenm2@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div style="word-wrap:break-word"><br><div><div>
On May 21, 2014, at 2:28 AM, Matt Arsenault <<a href="mailto:arsenm2@gmail.com" target="_blank">arsenm2@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div style="font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br><br>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.<br>
<br>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.<br>
<br></div></blockquote><br></div><div>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.</div>
</div></blockquote></div></blockquote></div></div></div></div></blockquote><div>(based on the original gv-const-addrspace code) the compiler loads literals for every element of global array to private memory and then chooses from there. It explains why I could not get it to work with bigger array than 32 elements.</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class="h5"><div>
<blockquote type="cite"><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
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.</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
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.</div>
</blockquote></div></div></div></div></blockquote><div>yeah, this patch is turning out not be the quick fix I thought it would, I'd say it's best to drop it for now.</div><div>I think it'd be best to just use vram for global variables (and constant address space), we can reuse code that exists for glbal AS*. Tom mentioned he might have some patches in this regard. </div>
<div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class="h5">
<br></div></div><div>Your test changes are also incorrect. Your check lines now use SI-CHECK/EG-CHECK, but the check prefixes are only SI / EG</div></div></blockquote><div> </div></div>I missed this part. I can fix it in v3, but I think it would be best to just drop this patch, and fix the tests when a proper solution for constant AS lands.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">regards,</div><div class="gmail_extra">Jan</div><div class="gmail_extra"><br></div><div class="gmail_extra">* const buff space on EG is too small for OpenCL FULL PROFILE anyway. OTOH, limiting this hw to EMBEDDED_PROFILE, so it can use const buffer for const as, would enable us to hide the rather slow int64 support behind an extension.</div>
</div>