From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id C9C083858D32 for ; Wed, 19 Jul 2023 08:49:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C9C083858D32 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 (m0353728.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 36J8CI9r021390; Wed, 19 Jul 2023 08:49:49 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=UVCZ53StYvahPsDBB8pZtSHfd4fOv4ehw4QLAgPIbFI=; b=bMR9DonGpm9qajwgfNFUXlvJgF7Xi13Y9wJK0HsLC4mlYLPNAum6F1HnK/7f9fg8xmlb 91U52xSLJx/rlX/KA83OikwzmwnH1522Sxy/NR5Sg95NRjwN5SO2efxkt3nW4ZwwnG+F Hn0Noev6bY5tHZlIxIiX+8K0kOT6WQSS9m9JyxOrENgEdVKRZNXxRB1/gUnQtBOOX++F bKxroSfgbHmzAqUz5e0VTtFU+uXXuQv8SEYdwUheVt7oNksys3Bq+n8SEN+g35enXchd jDxcFs/dEgMgqVs7q+PVGkHPFWkJGt0qjoa8Y9Ik4cp3wJ8wEMwUXNh+Ipj5LHVWgft7 Lw== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rxc3y1268-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Jul 2023 08:49:49 +0000 Received: from m0353728.ppops.net (m0353728.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 36J8nmWh001459; Wed, 19 Jul 2023 08:49:48 GMT Received: from ppma23.wdc07v.mail.ibm.com (5d.69.3da9.ip4.static.sl-reverse.com [169.61.105.93]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rxc3y125p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Jul 2023 08:49:48 +0000 Received: from pps.filterd (ppma23.wdc07v.mail.ibm.com [127.0.0.1]) by ppma23.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 36J6kMDG030668; Wed, 19 Jul 2023 08:49:47 GMT Received: from smtprelay01.fra02v.mail.ibm.com ([9.218.2.227]) by ppma23.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3rv79jpha7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Jul 2023 08:49:47 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay01.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 36J8njgU20251220 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 19 Jul 2023 08:49:45 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1B5182004B; Wed, 19 Jul 2023 08:49:45 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0726D20043; Wed, 19 Jul 2023 08:49:41 +0000 (GMT) Received: from [9.177.7.147] (unknown [9.177.7.147]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 19 Jul 2023 08:49:40 +0000 (GMT) Message-ID: Date: Wed, 19 Jul 2023 16:49:39 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: PING^2 [PATCH] Adjust the symbol for SECTION_LINK_ORDER linked_to section [PR99889] Content-Language: en-US To: Fangrui Song Cc: richard.sandiford@arm.com, Jakub Jelinek , Segher Boessenkool , GCC Patches , Peter Bergner , David Edelsohn , Richard Biener , "H.J. Lu" , AlanM , jlaw@ventanamicro.com References: <0558633c-b553-5ef1-aa6f-c76fcf297454@linux.ibm.com> <52ca56ad-af0f-598f-4ccf-aed61fce67b4@linux.ibm.com> <15b488a5-1f5e-c24e-be12-f402b0dcdb5e@linux.ibm.com> From: "Kewen.Lin" In-Reply-To: Content-Type: text/plain; charset=UTF-8 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: MNVQByz4vdCuj55lgNYebtSXRHoMea4F X-Proofpoint-ORIG-GUID: Q-cc0VFkXU1-iREPq-Arrn37qXRDRy_w Content-Transfer-Encoding: 8bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-07-19_04,2023-07-18_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1011 mlxscore=0 suspectscore=0 adultscore=0 spamscore=0 phishscore=0 malwarescore=0 mlxlogscore=999 priorityscore=1501 impostorscore=0 bulkscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2307190078 X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H5,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 Fangrui, on 2023/7/19 14:33, Fangrui Song wrote: > On Thu, Nov 24, 2022 at 7:26 PM Kewen.Lin via Gcc-patches > wrote: >> >> Hi Richard, >> >> on 2022/11/23 00:08, Richard Sandiford wrote: >>> "Kewen.Lin" writes: >>>> Hi Richard, >>>> >>>> Many thanks for your review comments! >>>> >>>>>>> on 2022/8/24 16:17, Kewen.Lin via Gcc-patches wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> As discussed in PR98125, -fpatchable-function-entry with >>>>>>>> SECTION_LINK_ORDER support doesn't work well on powerpc64 >>>>>>>> ELFv1 because the filled "Symbol" in >>>>>>>> >>>>>>>> .section name,"flags"o,@type,Symbol >>>>>>>> >>>>>>>> sits in .opd section instead of in the function_section >>>>>>>> like .text or named .text*. >>>>>>>> >>>>>>>> Since we already generates one label LPFE* which sits in >>>>>>>> function_section of current_function_decl, this patch is >>>>>>>> to reuse it as the symbol for the linked_to section. It >>>>>>>> avoids the above ABI specific issue when using the symbol >>>>>>>> concluded from current_function_decl. >>>>>>>> >>>>>>>> Besides, with this support some previous workarounds for >>>>>>>> powerpc64 ELFv1 can be reverted. >>>>>>>> >>>>>>>> btw, rs6000_print_patchable_function_entry can be dropped >>>>>>>> but there is another rs6000 patch which needs this rs6000 >>>>>>>> specific hook rs6000_print_patchable_function_entry, not >>>>>>>> sure which one gets landed first, so just leave it here. >>>>>>>> >>>>>>>> Bootstrapped and regtested on below: >>>>>>>> >>>>>>>> 1) powerpc64-linux-gnu P8 with default binutils 2.27 >>>>>>>> and latest binutils 2.39. >>>>>>>> 2) powerpc64le-linux-gnu P9 (default binutils 2.30). >>>>>>>> 3) powerpc64le-linux-gnu P10 (default binutils 2.30). >>>>>>>> 4) x86_64-redhat-linux with default binutils 2.30 >>>>>>>> and latest binutils 2.39. >>>>>>>> 5) aarch64-linux-gnu with default binutils 2.30 >>>>>>>> and latest binutils 2.39. >>>>>>>> >>>> >>>> [snip...] >>>> >>>>>>>> diff --git a/gcc/varasm.cc b/gcc/varasm.cc >>>>>>>> index 4db8506b106..d4de6e164ee 100644 >>>>>>>> --- a/gcc/varasm.cc >>>>>>>> +++ b/gcc/varasm.cc >>>>>>>> @@ -6906,11 +6906,16 @@ default_elf_asm_named_section (const char *name, unsigned int flags, >>>>>>>> fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE); >>>>>>>> if (flags & SECTION_LINK_ORDER) >>>>>>>> { >>>>>>>> - tree id = DECL_ASSEMBLER_NAME (decl); >>>>>>>> - ultimate_transparent_alias_target (&id); >>>>>>>> - const char *name = IDENTIFIER_POINTER (id); >>>>>>>> - name = targetm.strip_name_encoding (name); >>>>>>>> - fprintf (asm_out_file, ",%s", name); >>>>>>>> + /* For now, only section "__patchable_function_entries" >>>>>>>> + adopts flag SECTION_LINK_ORDER, internal label LPFE* >>>>>>>> + was emitted in default_print_patchable_function_entry, >>>>>>>> + just place it here for linked_to section. */ >>>>>>>> + gcc_assert (!strcmp (name, "__patchable_function_entries")); >>>>> >>>>> I like the idea of removing the rs600 workaround in favour of making the >>>>> target-independent more robust. But this seems a bit hackish. What >>>>> would we do if SECTION_LINK_ORDER was used for something else in future? >>>>> >>>> >>>> Good question! I think it depends on how we can get the symbol for the >>>> linked_to section, if adopting the name of the decl will suffer the >>>> similar issue which this patch wants to fix, we have to reuse the label >>>> LPFE* or some kind of new artificial label in the related section; or >>>> we can just go with the name of the given decl, or something related to >>>> that decl. Since we can't predict any future uses, I just placed an >>>> assertion here to ensure that we would revisit and adjust this part at >>>> that time. Does it sound reasonable to you? >>> >>> Yeah, I guess that's good enough. If the old scheme ends up being >>> correct for some future use, we can make the new behaviour conditional >>> on __patchable_function_entries. >> >> Yes, we can check if the given section name is >> "__patchable_function_entries". >> >>> >>> So yeah, the patch LGTM to me, thanks. >> >> Thanks again! I rebased and re-tested it on x86/aarch64/powerpc64{,le}, >> just committed in r13-4294-gf120196382ac5a. >> >> BR, >> Kewen > > Hi, Kewen, do you think whether your patch fixed > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110729 > (__patchable_function_entries has wrong sh_link) ? I just had a check and confirmed that it did fix the wrong sh_link, in the past it always uses the decl saved in named.decl (always f for the test case in PR110729), with this patch, it switches to use the label in its corresponding .text* (function section). > If yes, it may be useful to include some assembly tests... Right now > > rg '\.section.*__patchable' gcc/testsuite/ > > returns nothing. It's a good idea to add some testing coverage, I'm going to make a test case by checking the given ".section.*__patchable_function_entries.*,\.LPFE[012]". Thanks for the suggestion! BR, Kewen