On Tue, 2013-05-21 at 21:57 -0400, David Edelsohn wrote: > On Tue, May 21, 2013 at 7:13 PM, Sandra Loosemore > <san...@codesourcery.com> wrote: > > On 05/21/2013 04:04 PM, David Edelsohn wrote: > >> > >> > >> There are three issues here: > >> > >> 1) Someone in the LTC toolchain team needs to benchmark this patch on > >> POWER7. > > > > > > That would be great if somebody else could help with that. > > > > > >> 2) We need to clarify how the patch affects the ABI because it cannot > >> break the ABI. > > > > > > I understand this. > > > > > >> 3) Please stop saying that you cannot justify trying to get the patch > >> in mainline. Other developers have pointed out how the patch may be > >> incorrect. Do you want to deliver a broken compiler to CodeSourcery's > >> customers? The comment sets a bad tone for engaging with the GCC > >> community. > > > > > > I think you've misunderstood my position, here. Delivering a broken > > compiler is just what I want to avoid! We've had the original > > local-arrays-only patch in our local tree for a couple of years now, but we > > no longer have a customer for it. I thought the comments from the previous > > review would be straightforward to address and it would be worth making one > > more attempt to revise and resubmit the patch, but if the feedback we get > > from the community is that this is still broken in other ways and is going > > to need a lot more work before it's acceptable, we're going to give up on it > > and revert the previous version of the patch locally too. We have a lot of > > higher-priority patches in our local tree that we'd like to get on mainline, > > and limited resources for working on it, so we need to pick our battles. > > That's all. :-) > > I think the local arrays patch makes sense, if it does not hurt > performance. We had another recent case where increasing GCC's > knowledge about the alignment of memory returned by malloc allowed > additional vectorization opportunities, but hurt performance because > of bad spilling choices by GCC RA. This alignment patch may expose > similar RA problems. We may need to apply the patch with the > optimization disabled until the RA spilling problem is fixed. > > Increasing the alignment of arrays within structs and unions would be > nice, but that probably will change the ABI. I think that they best we > may be able to do is increase the alignment if the array is the first > element of the struct or union, see ROUND_TYPE_ALIGN for AIX. > Although this might be more trouble than it is worth. > > Pat or Bill, can you test the performance of the array alignment patch?
Sandra and David, The array-alignment patch is performance-neutral with respect to CPU2006. All variations were in the noise range. Thanks, Bill > > Thanks, David >
