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 F02873858D35 for ; Thu, 5 Jan 2023 04:04:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F02873858D35 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 Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 30502goJ011246; Thu, 5 Jan 2023 04:04:53 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=S6tEZs+7R7PNowlG6CyA+58kWx34JTdkpF7FBkFqiBE=; b=g/Je4Qwuqvc3IFkZOKqb0Pb0Rxm0H9+n0z8lx1c4EHBRExOptBkrQRMYNZBXsCi3w40m O00uubHZEtG7jPzNMLJibwuboGutjFlEarrR1cqWWVg2qW+rSysgMDNMbOOLoW3iRSUW eI99scoGKOoQ7ndMrthY505ry3HSBlfmNSFTDOKJ52TumLP309YVrrXgpI2wU0kPqXrf YeW9GkKfud5qKXoC077UEekcKpVyz4UIhD8WpYeWqTfAd4XgjKaURYixaC+mbt8oN6Po BkMW9zoEN95nbHzp6whGkFHMbTDBzlH/eHAhlrNd8GpeGgKQqSrfo3Xe9vG9WW26mbmW yQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3mwknpuy26-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 05 Jan 2023 04:04:53 +0000 Received: from m0127361.ppops.net (m0127361.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3053unbV001218; Thu, 5 Jan 2023 04:04:52 GMT Received: from ppma06ams.nl.ibm.com (66.31.33a9.ip4.static.sl-reverse.com [169.51.49.102]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3mwknpuy1p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 05 Jan 2023 04:04:52 +0000 Received: from pps.filterd (ppma06ams.nl.ibm.com [127.0.0.1]) by ppma06ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 304CiG0d000962; Thu, 5 Jan 2023 04:04:51 GMT Received: from smtprelay03.fra02v.mail.ibm.com ([9.218.2.224]) by ppma06ams.nl.ibm.com (PPS) with ESMTPS id 3mtcbfe5vb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 05 Jan 2023 04:04:50 +0000 Received: from smtpav06.fra02v.mail.ibm.com (smtpav06.fra02v.mail.ibm.com [10.20.54.105]) by smtprelay03.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 30544moj45613546 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 5 Jan 2023 04:04:48 GMT Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8D71520040; Thu, 5 Jan 2023 04:04:48 +0000 (GMT) Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EF64C20049; Thu, 5 Jan 2023 04:04:46 +0000 (GMT) Received: from [9.200.36.187] (unknown [9.200.36.187]) by smtpav06.fra02v.mail.ibm.com (Postfix) with ESMTP; Thu, 5 Jan 2023 04:04:46 +0000 (GMT) Message-ID: <2e48b4e9-7c50-7ccc-3534-e20576590dbc@linux.ibm.com> Date: Thu, 5 Jan 2023 12:04:45 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] rs6000: Don't use optimize_function_for_speed_p too early [PR108184] Content-Language: en-US To: Segher Boessenkool Cc: GCC Patches , David Edelsohn , Peter Bergner References: <197abd1f-081c-3206-4dd5-45f0b098612a@linux.ibm.com> <20230104104648.GI25951@gate.crashing.org> <20230104140226.GJ25951@gate.crashing.org> From: "Kewen.Lin" In-Reply-To: <20230104140226.GJ25951@gate.crashing.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: cQ8HgPsEOFE0i1UIhQ5EwTaCy5om5ihd X-Proofpoint-ORIG-GUID: iWgn2vWo1k8d1aCfscTtJpsw1eaEAuAl X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2023-01-04_07,2023-01-04_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 bulkscore=0 impostorscore=0 mlxlogscore=900 priorityscore=1501 suspectscore=0 mlxscore=0 adultscore=0 phishscore=0 clxscore=1015 lowpriorityscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2301050032 X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP 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: on 2023/1/4 22:02, Segher Boessenkool wrote: > Hi! > > On Wed, Jan 04, 2023 at 08:15:03PM +0800, Kewen.Lin wrote: >> on 2023/1/4 18:46, Segher Boessenkool wrote: >>>> @@ -25604,7 +25602,9 @@ 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 >>>> + && optimize_function_for_speed_p (cfun)) >>>> cfun->machine->save_toc_in_prologue = true; >>> >>> Is this correct? If so, it really needs a separate testcase. >> >> Yes, it just moves the condition from: >> >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -3978,8 +3978,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; >> >> here. > > That "just" reinforces that this really needs a testcase! It is all > action at a distance, none of this is trivial (if it was there would > not be a bug here in the first place, of course). OK, I'll make a test case for it. :) > >> I tried to find one test case before, but failed to find one which is not fragile >> to test. And I thought the associated test case has demonstrated why the use of >> optimize_function_for_{speed,size}_p is too early in function >> rs6000_option_override_internal, so I gave up then. Do you worry about that we >> could revert it unexpectedly in future and no sensitive test case is on it? > > I worry that it might contradict what some other code does. I also > worry that it just is not a sensible thing to do. > > I do not worry that your patch is not an improvement. But the resulting > code more clearly (than the original) is problematic. Where is r2 saved > to the frame if save_toc_in_prologue is false? If save_toc_in_prologue is false, the r2 saving to frame would occur at each indirect call. Currently separate shrink-wrapping will check save_toc_in_prologue to decide whether to consider saving toc as one component, I think that's why we enable save-toc-indirect implicitly (going to set save_toc_in_prologue) if it's not specified explicitly and doing separate shrink-wrapping. BR, Kewen