From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 1D92B3858D1E for ; Wed, 8 Nov 2023 02:51:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D92B3858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1D92B3858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699411879; cv=none; b=ooQ9n2Fk2APK9PICcK4n+OBhthRcEsgdU2LnTKk0LvQ+8ZdYXZnpulQfnrfqxl+kZ2maa6OyHXd8CmLaLMJ2ZSuBgHBp2kTx9y6NCCkpzBQ1u9BVj4Zsxe4ize3iPq/Y8Uz8CrdvRgYCTc8QVALVX6uwqqOYYxaEwEZpD5QtVKs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699411879; c=relaxed/simple; bh=7SqVDCTeKXAWK2adLI2W950oc0U/Jqopkdg+vH2qyxY=; h=DKIM-Signature:Message-ID:Date:Subject:From:To:MIME-Version; b=fzcz/szWf03qNa7NsmBWBH+eiVgyvjXJGdNDS85tSC/8R6qG6m7C1d6jbV+v0788CjfAQjwQjAaheSv9GL6mBjTPdl8bwQoUZ8oA5Ulu1bWyJsXbPC1ofZ6leiYAfTl3cpx+lJdCj1YXdGpIb+xNEkTbvOGsdMBkKuQI3nppzhU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353723.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3A82EiQV005748; Wed, 8 Nov 2023 02:51:16 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : from : to : cc : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=lOlwP7Y2+KHKOlZKBwrgaHTFFZCi8ceZ/M2ptzdrEZY=; b=WeALu/VqJTvd4EOqv3OskQmvuSkUIzI2oKwliTNaolBLtCRHGB0F1Fb6r548Q5ZQp0NP f8MMbBEyuFUHg0hAKPGQ3RlbWqexWkQRL+Zjnx0BzicE6QamTdM5eHatevmihZcr1KnT E3mUhpJIkHeeh7v6P7r59m9A0kpstM/9AdMODqkzySONifGRNEEfu36HE2tVQDFSfi2k CuI7mNSaSTXHulgderR064uj5wyx5p0aJtqty/AIDGEYLcVFn/uvYDL+Pv/R+rvlgVbh TUJvP/2l9cCfQhCd2SnayxRO2WycM/koO+Lam2khok87U79DWIkd6D94LDgEVAx2AFCA FQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3u81cf0y2n-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 08 Nov 2023 02:51:16 +0000 Received: from m0353723.ppops.net (m0353723.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3A82pFxd031124; Wed, 8 Nov 2023 02:51:15 GMT Received: from ppma13.dal12v.mail.ibm.com (dd.9e.1632.ip4.static.sl-reverse.com [50.22.158.221]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3u81cf0y22-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 08 Nov 2023 02:51:15 +0000 Received: from pps.filterd (ppma13.dal12v.mail.ibm.com [127.0.0.1]) by ppma13.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 3A82Eq7H003488; Wed, 8 Nov 2023 02:51:14 GMT Received: from smtprelay02.fra02v.mail.ibm.com ([9.218.2.226]) by ppma13.dal12v.mail.ibm.com (PPS) with ESMTPS id 3u7w21swsy-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 08 Nov 2023 02:51:14 +0000 Received: from smtpav05.fra02v.mail.ibm.com (smtpav05.fra02v.mail.ibm.com [10.20.54.104]) by smtprelay02.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 3A82pDAN28443152 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 8 Nov 2023 02:51:13 GMT Received: from smtpav05.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0BF6520040; Wed, 8 Nov 2023 02:51:13 +0000 (GMT) Received: from smtpav05.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0900E20043; Wed, 8 Nov 2023 02:51:11 +0000 (GMT) Received: from [9.177.74.45] (unknown [9.177.74.45]) by smtpav05.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 8 Nov 2023 02:51:10 +0000 (GMT) Message-ID: <38402e74-14db-9903-3dca-b4ef7c826505@linux.ibm.com> Date: Wed, 8 Nov 2023 10:51:08 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: PING^4 [PATCH v2] rs6000: Don't use optimize_function_for_speed_p too early [PR108184] Content-Language: en-US From: "Kewen.Lin" To: David Edelsohn , Segher Boessenkool Cc: Peter Bergner , GCC Patches References: <3be17163-63ed-5d86-99fd-c7c8401c2796@linux.ibm.com> <236d887a-ea18-3247-220e-ba56946318da@linux.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: 8lmIYKenxvGUtCFovNf7_A5wFhJspWxS X-Proofpoint-ORIG-GUID: RVOvlnTgcZI4GUs6Az1OZ2gFJP_HCavf Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-08_01,2023-11-07_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 impostorscore=0 lowpriorityscore=0 adultscore=0 mlxlogscore=999 clxscore=1015 suspectscore=0 bulkscore=0 priorityscore=1501 spamscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000 definitions=main-2311080022 X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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