Hi, Gentle ping this:
https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609993.html BR, Kewen > >>>> on 2023/1/16 17:08, Kewen.Lin via Gcc-patches wrote: >>>>> Hi, >>>>> >>>>> As Honza pointed out in [1], the current uses of function >>>>> optimize_function_for_speed_p in rs6000_option_override_internal >>>>> are too early, since the query results from the functions >>>>> optimize_function_for_{speed,size}_p could be changed later due >>>>> to profile feedback and some function attributes handlings etc. >>>>> >>>>> This patch is to move optimize_function_for_speed_p to all the >>>>> use places of the corresponding flags, which follows the existing >>>>> practices. Maybe we can cache it somewhere at an appropriate >>>>> timing, but that's another thing. >>>>> >>>>> Comparing with v1[2], this version added one test case for >>>>> SAVE_TOC_INDIRECT as Segher questioned and suggested, and it >>>>> also considered the possibility of explicit option (see test >>>>> cases pr108184-2.c and pr108184-4.c). I believe that excepting >>>>> for the intentional change on optimize_function_for_{speed, >>>>> size}_p, there is no other function change. >>>>> >>>>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607527.html >>>>> [2] https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609379.html >>>>> >>>>> Bootstrapped and regtested on powerpc64-linux-gnu P8, >>>>> powerpc64le-linux-gnu P{9,10} and powerpc-ibm-aix. >>>>> >>>>> Is it ok for trunk? >>>>> >>>>> BR, >>>>> Kewen >>>>> ----- >>>>> gcc/ChangeLog: >>>>> >>>>> * config/rs6000/rs6000.cc (rs6000_option_override_internal): Remove >>>>> all optimize_function_for_speed_p uses. >>>>> (fusion_gpr_load_p): Call optimize_function_for_speed_p along >>>>> with TARGET_P8_FUSION_SIGN. >>>>> (expand_fusion_gpr_load): Likewise. >>>>> (rs6000_call_aix): Call optimize_function_for_speed_p along with >>>>> TARGET_SAVE_TOC_INDIRECT. >>>>> * config/rs6000/predicates.md (fusion_gpr_mem_load): Call >>>>> optimize_function_for_speed_p along with TARGET_P8_FUSION_SIGN. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> * gcc.target/powerpc/pr108184-1.c: New test. >>>>> * gcc.target/powerpc/pr108184-2.c: New test. >>>>> * gcc.target/powerpc/pr108184-3.c: New test. >>>>> * gcc.target/powerpc/pr108184-4.c: New test. >>>>> --- >>>>> gcc/config/rs6000/predicates.md | 5 +++- >>>>> gcc/config/rs6000/rs6000.cc | 19 +++++++++----- >>>>> gcc/testsuite/gcc.target/powerpc/pr108184-1.c | 16 ++++++++++++ >>>>> gcc/testsuite/gcc.target/powerpc/pr108184-2.c | 15 +++++++++++ >>>>> gcc/testsuite/gcc.target/powerpc/pr108184-3.c | 25 +++++++++++++++++++ >>>>> gcc/testsuite/gcc.target/powerpc/pr108184-4.c | 24 ++++++++++++++++++ >>>>> 6 files changed, 97 insertions(+), 7 deletions(-) >>>>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr108184-1.c >>>>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr108184-2.c >>>>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr108184-3.c >>>>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr108184-4.c >>>>> >>>>> diff --git a/gcc/config/rs6000/predicates.md >>>>> b/gcc/config/rs6000/predicates.md >>>>> index a1764018545..9f84468db84 100644 >>>>> --- a/gcc/config/rs6000/predicates.md >>>>> +++ b/gcc/config/rs6000/predicates.md >>>>> @@ -1878,7 +1878,10 @@ (define_predicate "fusion_gpr_mem_load" >>>>> >>>>> /* Handle sign/zero extend. */ >>>>> if (GET_CODE (op) == ZERO_EXTEND >>>>> - || (TARGET_P8_FUSION_SIGN && GET_CODE (op) == SIGN_EXTEND)) >>>>> + || (TARGET_P8_FUSION_SIGN >>>>> + && GET_CODE (op) == SIGN_EXTEND >>>>> + && (rs6000_isa_flags_explicit & OPTION_MASK_P8_FUSION_SIGN >>>>> + || optimize_function_for_speed_p (cfun)))) >>>>> { >>>>> op = XEXP (op, 0); >>>>> mode = GET_MODE (op); >>>>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >>>>> index 6ac3adcec6b..f47d21980a9 100644 >>>>> --- a/gcc/config/rs6000/rs6000.cc >>>>> +++ b/gcc/config/rs6000/rs6000.cc >>>>> @@ -3997,8 +3997,7 @@ rs6000_option_override_internal (bool global_init_p) >>>>> /* If we can shrink-wrap the TOC register save separately, then use >>>>> -msave-toc-indirect unless explicitly disabled. */ >>>>> if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0 >>>>> - && flag_shrink_wrap_separate >>>>> - && optimize_function_for_speed_p (cfun)) >>>>> + && flag_shrink_wrap_separate) >>>>> rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT; >>>>> >>>>> /* Enable power8 fusion if we are tuning for power8, even if we aren't >>>>> @@ -4032,7 +4031,6 @@ rs6000_option_override_internal (bool global_init_p) >>>>> zero extending load, and an explicit sign extension. */ >>>>> if (TARGET_P8_FUSION >>>>> && !(rs6000_isa_flags_explicit & OPTION_MASK_P8_FUSION_SIGN) >>>>> - && optimize_function_for_speed_p (cfun) >>>>> && optimize >= 3) >>>>> rs6000_isa_flags |= OPTION_MASK_P8_FUSION_SIGN; >>>>> >>>>> @@ -25690,7 +25688,10 @@ rs6000_call_aix (rtx value, rtx func_desc, rtx >>>>> tlsarg, rtx cookie) >>>>> >>>>> /* Can we optimize saving the TOC in the prologue or >>>>> do we need to do it at every call? */ >>>>> - if (TARGET_SAVE_TOC_INDIRECT && !cfun->calls_alloca) >>>>> + if (TARGET_SAVE_TOC_INDIRECT >>>>> + && !cfun->calls_alloca >>>>> + && (rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT >>>>> + || optimize_function_for_speed_p (cfun))) >>>>> cfun->machine->save_toc_in_prologue = true; >>>>> else >>>>> { >>>>> @@ -27557,7 +27558,10 @@ fusion_gpr_load_p (rtx addis_reg, /* >>>>> register set via addis. */ >>>>> >>>>> /* Allow sign/zero extension. */ >>>>> if (GET_CODE (mem) == ZERO_EXTEND >>>>> - || (GET_CODE (mem) == SIGN_EXTEND && TARGET_P8_FUSION_SIGN)) >>>>> + || (GET_CODE (mem) == SIGN_EXTEND >>>>> + && TARGET_P8_FUSION_SIGN >>>>> + && (rs6000_isa_flags_explicit & OPTION_MASK_P8_FUSION_SIGN >>>>> + || optimize_function_for_speed_p (cfun)))) >>>>> mem = XEXP (mem, 0); >>>>> >>>>> if (!MEM_P (mem)) >>>>> @@ -27621,7 +27625,10 @@ expand_fusion_gpr_load (rtx *operands) >>>>> enum rtx_code extend = UNKNOWN; >>>>> >>>>> if (GET_CODE (orig_mem) == ZERO_EXTEND >>>>> - || (TARGET_P8_FUSION_SIGN && GET_CODE (orig_mem) == SIGN_EXTEND)) >>>>> + || (TARGET_P8_FUSION_SIGN >>>>> + && GET_CODE (orig_mem) == SIGN_EXTEND >>>>> + && (rs6000_isa_flags_explicit & OPTION_MASK_P8_FUSION_SIGN >>>>> + || optimize_function_for_speed_p (cfun)))) >>>>> { >>>>> extend = GET_CODE (orig_mem); >>>>> orig_mem = XEXP (orig_mem, 0); >>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr108184-1.c >>>>> b/gcc/testsuite/gcc.target/powerpc/pr108184-1.c >>>>> new file mode 100644 >>>>> index 00000000000..eae3bb9b7c4 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr108184-1.c >>>>> @@ -0,0 +1,16 @@ >>>>> +/* Only possible to fuse sign extended loads with the addis when >>>>> + optimize >= 3 and Power8 fusion takes effects. -mno-prefixed >>>>> + ensures test point isn't broken (due to prefixed plha). */ >>>>> +/* { dg-options "-mdejagnu-tune=power8 -O3 -mno-prefixed" } */ >>>>> + >>>>> +/* Verify it doesn't optimize this function for speed as it's cold, >>>>> + by checking it doesn't try to fuse sign extended loads with addis, >>>>> + which results in a zero extended load and a sign extension. */ >>>>> + >>>>> +__attribute__ ((cold)) int >>>>> +fusion_short (short *p) >>>>> +{ >>>>> + return p[0x12345]; >>>>> +} >>>>> + >>>>> +/* { dg-final { scan-assembler-not {\mextsh\M} } } */ >>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr108184-2.c >>>>> b/gcc/testsuite/gcc.target/powerpc/pr108184-2.c >>>>> new file mode 100644 >>>>> index 00000000000..9f703f7fc6b >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr108184-2.c >>>>> @@ -0,0 +1,15 @@ >>>>> +/* -mno-prefixed ensures test point isn't broken (due to prefixed plha). >>>>> */ >>>>> +/* { dg-options "-mdejagnu-tune=power8 -O2 -mpower8-fusion-sign >>>>> -mno-prefixed" } */ >>>>> + >>>>> +/* Verify the explicit option -mpower8-fusion-sign can still fuse >>>>> + sign extended loads with addis, which results in a zero extended >>>>> + load and a sign extension. It means the cold attribute which >>>>> + indicates to optimize for size doesn't affect. */ >>>>> + >>>>> +__attribute__ ((cold)) int >>>>> +fusion_short (short *p) >>>>> +{ >>>>> + return p[0x12345]; >>>>> +} >>>>> + >>>>> +/* { dg-final { scan-assembler-times {\mextsh\M} 1 } } */ >>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr108184-3.c >>>>> b/gcc/testsuite/gcc.target/powerpc/pr108184-3.c >>>>> new file mode 100644 >>>>> index 00000000000..ceaa96e4421 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr108184-3.c >>>>> @@ -0,0 +1,25 @@ >>>>> +/* { dg-do compile { target { powerpc*-*-aix* powerpc*-*-linux* } } } */ >>>>> +/* { dg-require-effective-target fpic } */ >>>>> +/* { dg-options "-fpic -mno-pcrel -O2" } */ >>>>> + >>>>> +/* Verify it doesn't imply -msave-toc-indirect, so >>>>> + it doesn't take effect and we have two separated >>>>> + toc savings for these two long calls. */ >>>>> + >>>>> +void foo (void) __attribute__((__longcall__)); >>>>> +int baz (void) __attribute__((__longcall__)); >>>>> + >>>>> +__attribute__ ((cold)) int >>>>> +bar (void) >>>>> +{ >>>>> + foo (); >>>>> + return baz () + 1; >>>>> +} >>>>> + >>>>> +/* Linux LE */ >>>>> +/* { dg-final { scan-assembler-times {\mstd 2,24\(1\)} 2 { target >>>>> powerpc_elfv2 } } } */ >>>>> +/* Linux BE 64 bit or AIX 64 bit */ >>>>> +/* { dg-final { scan-assembler-times {\mstd 2,40\(1\)} 2 { target { lp64 >>>>> && { ! powerpc_elfv2 } } } } } */ >>>>> +/* AIX 32 bit */ >>>>> +/* { dg-final { scan-assembler-times {\mstw 2,20\(1\)} 2 { target { >>>>> ilp32 && powerpc*-*-aix* } } } } */ >>>>> + >>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr108184-4.c >>>>> b/gcc/testsuite/gcc.target/powerpc/pr108184-4.c >>>>> new file mode 100644 >>>>> index 00000000000..2a70a603047 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr108184-4.c >>>>> @@ -0,0 +1,24 @@ >>>>> +/* { dg-do compile { target { powerpc*-*-aix* powerpc*-*-linux* } } } */ >>>>> +/* { dg-require-effective-target fpic } */ >>>>> +/* { dg-options "-fpic -mno-pcrel -O2 -msave-toc-indirect" } */ >>>>> + >>>>> +/* Verify the explicit -msave-toc-indirect always >>>>> + takes effect, so there is only one toc saving. */ >>>>> + >>>>> +void foo (void) __attribute__((__longcall__)); >>>>> +int baz (void) __attribute__((__longcall__)); >>>>> + >>>>> +__attribute__ ((cold)) int >>>>> +bar (void) >>>>> +{ >>>>> + foo (); >>>>> + return baz () + 1; >>>>> +} >>>>> + >>>>> +/* Linux LE */ >>>>> +/* { dg-final { scan-assembler-times {\mstd 2,24\(1\)} 1 { target >>>>> powerpc_elfv2 } } } */ >>>>> +/* Linux BE 64 bit or AIX 64 bit */ >>>>> +/* { dg-final { scan-assembler-times {\mstd 2,40\(1\)} 1 { target { lp64 >>>>> && { ! powerpc_elfv2 } } } } } */ >>>>> +/* AIX 32 bit */ >>>>> +/* { dg-final { scan-assembler-times {\mstw 2,20\(1\)} 1 { target { >>>>> ilp32 && powerpc*-*-aix* } } } } */ >>>>> + >>>>> -- >>>>> 2.37.0
