This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] new target hook for loop unrolling adjustment
- From: Christian Borntraeger <borntraeger at de dot ibm dot com>
- To: Richard Guenther <richard dot guenther at gmail dot com>, Wolfgang Gellerich <gellerich at de dot ibm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Andreas Krebbel <Andreas dot Krebbel at de dot ibm dot com>
- Date: Mon, 25 Jan 2010 14:34:51 +0100
- Subject: Re: [PATCH] new target hook for loop unrolling adjustment
- References: <201001250947.46123.borntraeger@de.ibm.com> <84fc9c001001250516u5a0a2349v21edac25f57059a8@mail.gmail.com>
Am Montag 25 Januar 2010 14:16:08 schrieb Richard Guenther:
> > + basic_block *bbs = get_loop_body (loop);
>
> You need to free bbs, and better not allocate them before ...
Agreed.
[...]
> > + /* Only z10 needs special handling. */
> > + if (s390_tune != PROCESSOR_2097_Z10)
> > + return nunroll;
>
> ... you actually use them.
Agreed.
[...]
> > + return MIN (nunroll, (unsigned) (Z10_DPU_SLOTS * 0.9) / mem_count);
> > + case 2:
> > + return MIN (nunroll, (unsigned) (Z10_DPU_SLOTS * 0.7) / mem_count);
> > + default:
> > + return MIN (nunroll, (unsigned) (Z10_DPU_SLOTS * 0.5) / mem_count);
>
> Do not use floating-point arithmetic on the host.
Ok.
>
> I don't think this does what you want. loop_depth is the
> number of superloops (and you should use loop_depth (loop),
> not loop_depth of the first basic-block). You want to check
> if loop is an innermost loop (from your description of the patch).
>
> Thus, I'd simply do
>
> if (!loop->inner)
> return nunroll;
>
> at the start of the function and only do the same adjustment
> for all other loops.
>
> Thus, I think your measurements if you've done any are bogus.
We have done measurements and we have seen improvements.
I will double check and discuss the necessary changes with Wolfgang.
Thank you for the feedback.
Christian