<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 30, 2014 at 2:08 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000"><div><div class="h5">
    <br>
    <div>On 10/28/2014 09:27 AM, Chandler
      Carruth wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra"><br>
          <div class="gmail_quote">On Tue, Oct 28, 2014 at 6:55 AM, Hal
            Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div style="overflow:hidden">Chandler,<br>
                <br>
                Can you please look at this? How do you think we should
                canonicalize this (is this the right approach)?</div>
            </blockquote>
          </div>
          <br>
          Oof... yea.</div>
        <div class="gmail_extra"><br>
        </div>
        <div class="gmail_extra">I wish I were more confident of what
          the "right" answer is here any more. =[</div>
      </div>
    </blockquote></div></div>
    At the dev conference, Hal and I talked about a couple of options. 
    I'm sure which is the "right" one, so let me lay out what I remember
    of that discussion.<br>
    <br>
    Option 1 - Leave the load alone, improve GVN <br>
    - not necessarily a bad option, but slightly risky in that it
    involves changes to key infrastructure with little in tree
    motivation<br>
    - the original change doing this was rejected<br>
    <br>
    Option 2 - Transform to load of component element types<br>
    - tricky to get layout exactly right, but definitely doable<br>
    <br>
    Option 3 - Transform to load of iN where N is sizeof(agg)*8.  <br>
    <br>
    Option 4 - Transform to series of smaller integer loads<br>
    - This appears to be what this patch implements.  Not entirely sure
    why this was chosen.  <br>
    <br>
    Option 5 - Transform to alloca and memcpy<br>
    - Clang appears to emit loads of structs via an alloca (for the
    local) and a memcpy.  The optimizer deconstructs this where
    appropriate.  <br>
    - I have no idea why Clang chose this option.  My best guess is to
    exploit information about POD types?<br>
    <br>
    Personally, I'd lean towards 5,1,2 above (in roughly that order).  1
    & 2 seem like better long term solutions, but 5 probably works
    fairly well today.  I'm not really a fan of either 3 or 4 since we
    loose things like distinctions between pointers and integers.  If we
    had to choose, I'd take 3 over 4.  <br>
    <br>
    I think we also discussed the trade off between a pass and an
    instcombine transform.  I would lean towards making whichever option
    we chose a canonicalization rule in instcombine.<br>
    <br>
    Also, this patch implements option 2 for a struct with a single
    element type which seems like a (independently) useful
    canonicalization.  Should we introduce that transform as a
    canonicalization in instcombine?  I'd lean towards that.  <br>
    <br>
    Chandler, Hal - Thoughts, opinions?  <br></div></blockquote><div><br></div><div>2 seems like the best option to me. When you load and store an FCA, you don't get to copy the padding of the struct with you. Once it's loaded, each element is its own value. There's no way to recover the padding. memcpy doesn't represent this.</div></div></div></div>