<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 05/21/2014 11:25 AM, Jan Vesely
      wrote:<br>
    </div>
    <blockquote
cite="mid:CABE_ZV1nFHYOhOsGZZ7sax9x=3jo0aM9vgOVcyvDyOqRX1SCnA@mail.gmail.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html;
        charset=ISO-8859-1">
      <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
                moz-do-not-send="true" 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 moz-do-not-send="true"
                          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 moz-do-not-send="true"
                              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
                                    moz-do-not-send="true"
                                    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. <br>
            </div>
          </div>
        </div>
      </div>
    </blockquote>
    Yes, I would drop it. I'll commit the test case parts<br>
    <br>
    <br>
    <blockquote
cite="mid:CABE_ZV1nFHYOhOsGZZ7sax9x=3jo0aM9vgOVcyvDyOqRX1SCnA@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <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>
      <br>
    </blockquote>
    Constant initializers should be fine to handle in the constant
    buffers since if it doesn't fit you can fall back to global, and the
    common case is a small initializer. For constant pointer arguments,
    you have to use global and there is an AMD extension
    max_constant_size attribute you can place on constant pointers to
    allow assuming they are small enough to fit<br>
  </body>
</html>