| Message ID | CAD57uCdA=v4=8+8fCybrNP5Es5jpt0eztVYTqf6Pe2BP_tcQ1w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 11/27/13 10:30, Yvan Roux wrote: >> Please include either the patch you are pinging or at the least a link to it >> in the archives. > > Ok, sorry for that, here is the patch and Changelog > > Yvan > > > 2013-11-17 Yvan Roux <yvan.roux@linaro.org> > > * config/arm/arm.md (store_minmaxsi): Use only when > optimize_function_for_size_p. Thanks. This is fine for the trunk. And yes, having an insn's validity change based on what block it's in is most definitely bad. I was a bit concerned have the x86 backend, as I happened to know it uses optimize_insn_for_* rather extensively. But thankfully it's only used in splitters, expanders & peephole2 patterns, which should all be safe. If you wouldn't mind, could you look at md.texi and see if there's a reasonable place to put verbage about this issue in the internals manual? It might save someone time in the future :-) Thanks, Jeff
> On 11/27/13 10:30, Yvan Roux wrote: > >>Please include either the patch you are pinging or at the least a link to it > >>in the archives. > > > >Ok, sorry for that, here is the patch and Changelog > > > >Yvan > > > > > >2013-11-17 Yvan Roux <yvan.roux@linaro.org> > > > > * config/arm/arm.md (store_minmaxsi): Use only when > > optimize_function_for_size_p. > Thanks. > > This is fine for the trunk. And yes, having an insn's validity > change based on what block it's in is most definitely bad. > > I was a bit concerned have the x86 backend, as I happened to know it > uses optimize_insn_for_* rather extensively. But thankfully it's > only used in splitters, expanders & peephole2 patterns, which should > all be safe. Yep, this was (is) the plan - have instructions independent on profile (since they can be freely moved from hot parts to cold and in weird cases also from cold to hot) while expanders/splitters and peep2s may use profile to choose appropriate generation strategy. I tried to review backend for places where splitters/peeps/expanders are used and set profile according reset it to default for the possible cases where we call them from profile unaware pass. Honza > > If you wouldn't mind, could you look at md.texi and see if there's a > reasonable place to put verbage about this issue in the internals > manual? It might save someone time in the future :-) > > Thanks, > Jeff
On 27 November 2013 19:13, Jeff Law <law@redhat.com> wrote: > On 11/27/13 10:30, Yvan Roux wrote: >>> >>> Please include either the patch you are pinging or at the least a link to >>> it >>> in the archives. >> >> >> Ok, sorry for that, here is the patch and Changelog >> >> Yvan >> >> >> 2013-11-17 Yvan Roux <yvan.roux@linaro.org> >> >> * config/arm/arm.md (store_minmaxsi): Use only when >> optimize_function_for_size_p. > > Thanks. > > This is fine for the trunk. And yes, having an insn's validity change based > on what block it's in is most definitely bad. > > I was a bit concerned have the x86 backend, as I happened to know it uses > optimize_insn_for_* rather extensively. But thankfully it's only used in > splitters, expanders & peephole2 patterns, which should all be safe. I did the same on ARM and only found one in a peephole2 pattern. > If you wouldn't mind, could you look at md.texi and see if there's a > reasonable place to put verbage about this issue in the internals manual? > It might save someone time in the future :-) Sure, I'll look at that. Thanks, Yvan > Thanks, > 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]);