[PATCH][LoopVectorizer] Restrict the unroll factor of reductions in loops

[Gerolf Hoflehner]

I looked at some of the regressions. There are at least two reasons for them due to the larger unrolling factor: first, unrolling factor can increase resource pressure and fill up reservation station buffers. Second, loops may no longer fit into the loop buffer and lose the decoding boost. We will need to hook up the unroll factor to better machine modeling to catch these cases. With gains and losses in balance I’m still slightly in favor the committed code, but we’ll have to revisit. 

Cheers
Gerolf

On Aug 20, 2014, at 5:13 PM, James Molloy <james.molloy at arm.com> wrote:

> Hi All,
> 
> Thanks, committed as r21614[01].
> 
> Gerolf, I didn’t do specific checking of compile time for this change because it is just adding a bailout condition – I can’t see how that could significantly alter compile time.
> 
> Cheers, and sorry for the high latency on this one,
> 
> James
> 
> From: Gerolf Hoflehner <ghoflehner at apple.com>
> Date: Wednesday, 13 August 2014 21:25
> To: Arnold Schwaighofer <aschwaighofer at apple.com>, Admin <james.molloy at arm.com>
> Cc: James Molloy <james at jamesmolloy.co.uk>, Hal Finkel <hfinkel at anl.gov>, Admin <james.molloy at arm.com>, "llvm-commits at cs.uiuc.edu" <llvm-commits at cs.uiuc.edu>
> Subject: Re: [PATCH][LoopVectorizer] Restrict the unroll factor of reductions in loops
> 
> There are a number of regression (lowercase -40%, link pack ~-5%) and gains (puzzle? >30%, sgefa > 13%). I’ll do some analysis in the next couple of days and follow up with bug filings as necessary. So go for the gains for now and trust that we’ll recover the losses.
> 
> I haven’t done any compile-time measurements for the patch. Did you cover it with high confidence?
> 
> Thanks
> Gerolf
> 
> 
> 
> On Aug 12, 2014, at 10:19 AM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> 
>> LGTM.
>> 
>> I did see some regressions running the whole test-suite on the two patches (+ "ST->isCyclone()") we (on our end) might want to look into - I’ll send my results to Gerolf.
>> 
>> 
>> Thanks,
>> Arnold
>> 
>> 
>>> On Aug 11, 2014, at 9:17 AM, James Molloy <james at jamesmolloy.co.uk> wrote:
>>> 
>>> Hi Arnold, Gerolf, Hal,
>>> 
>>> Good idea about a tuning option. Attached is a patch that implements that tuning option. Is it OK to commit?
>>> 
>>> Gerolf, did you want me to add "|| isCyclone()" to AArch64TargetTransformInfo?
>>> 
>>> Cheers,
>>> 
>>> James
>>> 
> 
> 
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> 
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

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