Add a pass to convert aggregate loads/stores into scalar load stores

[Philip Reames]

On 10/28/2014 09:27 AM, Chandler Carruth wrote:
>
> On Tue, Oct 28, 2014 at 6:55 AM, Hal Finkel <hfinkel at anl.gov 
> <mailto:hfinkel at anl.gov>> wrote:
>
>     Chandler,
>
>     Can you please look at this? How do you think we should
>     canonicalize this (is this the right approach)?
>
>
> Oof... yea.
>
> I wish I were more confident of what the "right" answer is here any 
> more. =[
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.

Option 1 - Leave the load alone, improve GVN
- not necessarily a bad option, but slightly risky in that it involves 
changes to key infrastructure with little in tree motivation
- the original change doing this was rejected

Option 2 - Transform to load of component element types
- tricky to get layout exactly right, but definitely doable

Option 3 - Transform to load of iN where N is sizeof(agg)*8.

Option 4 - Transform to series of smaller integer loads
- This appears to be what this patch implements.  Not entirely sure why 
this was chosen.

Option 5 - Transform to alloca and memcpy
- Clang appears to emit loads of structs via an alloca (for the local) 
and a memcpy.  The optimizer deconstructs this where appropriate.
- I have no idea why Clang chose this option.  My best guess is to 
exploit information about POD types?

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.

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.

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.

Chandler, Hal - Thoughts, opinions?


>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

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