<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<div>
<div>Good point. You can add struct tbaa to the memcpy to preserve padding.</div>
<div><br>
</div>
<div>-Hal</div>
<div><br>
</div>
<div><font style="color:#333333"><i>Sent from my Verizon Wireless 4G LTE DROID</i></font></div>
</div>
<br>
<br>
Reid Kleckner <rnk@google.com> wrote:<br>
<br>
<div>
<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">
<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>
</div>
</body>
</html>