| Message ID | CAD57uCcbuZaZNmozZ36+kgOqOS2EWjvsEX2qXrwjojeeB03raw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
So, the validation is ok with this patch, I'm just not able to say if
the original performance issue is still fixed with it. Could you
check it Kyrylo ?
Yvan
2013-11-17 Yvan Roux <yvan.roux@linaro.org>
* config/arm/arm.md (store_minmaxsi): Use only when
optimize_function_for_size_p.
On 15 November 2013 15:59, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Sometimes 4 will be needed, since both original register values may
>> remain live.
>
> Indeed.
>
>> However, I'm inclined to agree that while it should be possible to
>> decide at the *function* level whether or not an insn is valid, doing so
>> at the block level is probably unsafe.
>
> Ok, so the attached patch should fix the issue, its validation is ongoing.
On 18/11/13 08:37, Yvan Roux wrote: > So, the validation is ok with this patch, I'm just not able to say if > the original performance issue is still fixed with it. Could you > check it Kyrylo ? Hi Yvan, I'll run the benchmark today to confirm the performance, but from compiling some code sequences that exhibited the bad behaviour in the past, I see that this patch still fixes the issues. store_minmaxsi is not generated when optimising for speed. Thanks, Kyrill > > Yvan > > 2013-11-17 Yvan Roux <yvan.roux@linaro.org> > > * config/arm/arm.md (store_minmaxsi): Use only when > optimize_function_for_size_p. > > > On 15 November 2013 15:59, Yvan Roux <yvan.roux@linaro.org> wrote: >>> Sometimes 4 will be needed, since both original register values may >>> remain live. >> Indeed. >> >>> However, I'm inclined to agree that while it should be possible to >>> decide at the *function* level whether or not an insn is valid, doing so >>> at the block level is probably unsafe. >> Ok, so the attached patch should fix the issue, its validation is ongoing.
> Hi Yvan, > I'll run the benchmark today to confirm the performance, but from compiling > some code sequences that exhibited the bad behaviour in the past, I see that > this patch still fixes the issues. store_minmaxsi is not generated when > optimising for speed. Ok Cool, Thanks Kyrill Cheers, Yvan
On 18/11/13 09:14, Kyrill Tkachov wrote: > On 18/11/13 08:37, Yvan Roux wrote: >> So, the validation is ok with this patch, I'm just not able to say if >> the original performance issue is still fixed with it. Could you >> check it Kyrylo ? > Hi Yvan, > I'll run the benchmark today to confirm the performance, yep, all good performance-wise :) Thanks, Kyrill > but from compiling some > code sequences that exhibited the bad behaviour in the past, I see that this > patch still fixes the issues. store_minmaxsi is not generated when optimising > for speed. > > > Thanks, > Kyrill > >
> yep, all good performance-wise :)
Great, Thanks Kyrill.
Ok for trunk ?
Yvan
Ping. On 19 November 2013 09:52, Yvan Roux <yvan.roux@linaro.org> wrote: >> yep, all good performance-wise :) > > Great, Thanks Kyrill. > > Ok for trunk ? > > Yvan
On 11/27/13 03:18, Yvan Roux wrote: > Ping. > > On 19 November 2013 09:52, Yvan Roux <yvan.roux@linaro.org> wrote: >>> yep, all good performance-wise :) >> >> Great, Thanks Kyrill. >> >> Ok for trunk ? Please include either the patch you are pinging or at the least a link to it in the archives. jeff
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index e8d5464..da387fb 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -3642,7 +3642,7 @@ [(match_operand:SI 1 "s_register_operand" "r") (match_operand:SI 2 "s_register_operand" "r")])) (clobber (reg:CC CC_REGNUM))] - "TARGET_32BIT && optimize_insn_for_size_p()" + "TARGET_32BIT && optimize_function_for_size_p (cfun)" "* operands[3] = gen_rtx_fmt_ee (minmax_code (operands[3]), SImode, operands[1], operands[2]);