Message ID | 20221202175225.2780-3-cupertino.miranda@oracle.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6EAAD385782B for <patchwork@sourceware.org>; Fri, 2 Dec 2022 17:53:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6EAAD385782B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1670003610; bh=hxyLaTdzgEcUQe9fvBFFO0kQ73Tt2lUKYTzZEdP1ICU=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=QjC+W0E0mjhEKS83LFfHIUaY89MJhjbnc/LV2BXFtvixlg7Y3ZCBTgflaR4Q7jde9 iPqTkyUBD03fwMN3S+giJrXPznDiUYEKXDewthOnN5A8uBq7lEgy3LmuRa9iBN0e/l jvoXbPmzi2hvHBBtwyiWT2CEeX8MO6mqv30asdPY= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0b-00069f02.pphosted.com (mx0b-00069f02.pphosted.com [205.220.177.32]) by sourceware.org (Postfix) with ESMTPS id EBD8A385840E for <gcc-patches@gcc.gnu.org>; Fri, 2 Dec 2022 17:53:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EBD8A385840E Received: from pps.filterd (m0246632.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2B2FhjPm003707 for <gcc-patches@gcc.gnu.org>; Fri, 2 Dec 2022 17:53:00 GMT Received: from phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta01.appoci.oracle.com [138.1.114.2]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3m782ha4xx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gcc-patches@gcc.gnu.org>; Fri, 02 Dec 2022 17:53:00 +0000 Received: from pps.filterd (phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 2B2HeTga018439 for <gcc-patches@gcc.gnu.org>; Fri, 2 Dec 2022 17:52:59 GMT Received: from nam10-dm6-obe.outbound.protection.outlook.com (mail-dm6nam10lp2101.outbound.protection.outlook.com [104.47.58.101]) by phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTPS id 3m4a2py1n9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gcc-patches@gcc.gnu.org>; Fri, 02 Dec 2022 17:52:59 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gUFH7aqABjBatkydS1Y8rjGJxWs5YUb2w8KdvPz0PX28cE0xA/aDDYPAwPO+rG5YD2j9WSpU6bEo3nPZvv7WDTyFvT8zGy4rhGVC6u20/spgUVCOCpaRr8PwRTtkHH4XgI1NIdznoiz/97VwnROCA5gulu8iNoAudVH1i8sSxE3hhEzBAudQg3uonE1VCc3/CHPtrqLrH/bHSVufP1A9gZP+2NYBnM6+eVGTrcmQr2SbFwCXUNKcRBYsKmdBu82EhrfmCXzrQQjeadGJ3SXthGtyYlaFCMZUPTkiQq6sjPpadMHxTHDvXxl/jfn3G8e/vFAISBgBUS0JlVYWiuPVwg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=hxyLaTdzgEcUQe9fvBFFO0kQ73Tt2lUKYTzZEdP1ICU=; b=bRSg72Uw+D22EEHYCKjyGSNux+his0iK293GU1lPxX27YdDs0VZC+6aRcQUgNghf+LnviA9oZ96SNH1WZj9KZwfBp1YdzL7x3sUrF2Smo97tQgB0OOgJ981K6traGFTGgYGUckzjZkxcS9bGnwEnWWkLGj3negEdPVCXCUUg9xZt4kVghgAxteT3oRZhT3dv6HpaLdbj06reRlDvW4S1QmEN9IyKAMvS6CH2VcIYxD1UgYMuW4W6HkZKyx7vKdYQjSqh1VO/fqkaDbm8feRcqPOZ4uiQBjYtiSc84xjFIP2YjYuJYNacvOt8WWpkFB7ywKXei0Lkd0giaSQoIX85lQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none Received: from BN6PR1001MB2340.namprd10.prod.outlook.com (2603:10b6:405:30::36) by SJ2PR10MB7014.namprd10.prod.outlook.com (2603:10b6:a03:4c2::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.10; Fri, 2 Dec 2022 17:52:57 +0000 Received: from BN6PR1001MB2340.namprd10.prod.outlook.com ([fe80::8681:5931:e558:2638]) by BN6PR1001MB2340.namprd10.prod.outlook.com ([fe80::8681:5931:e558:2638%7]) with mapi id 15.20.5857.023; Fri, 2 Dec 2022 17:52:56 +0000 To: gcc-patches@gcc.gnu.org Cc: cupertino.miranda@oracle.com, jose.marchesi@oracle.com Subject: [PATCH 2/2] Corrected pr25521.c target matching. Date: Fri, 2 Dec 2022 17:52:25 +0000 Message-Id: <20221202175225.2780-3-cupertino.miranda@oracle.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221202175225.2780-1-cupertino.miranda@oracle.com> References: <20221202175225.2780-1-cupertino.miranda@oracle.com> Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: LO2P265CA0205.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:9e::25) To BN6PR1001MB2340.namprd10.prod.outlook.com (2603:10b6:405:30::36) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN6PR1001MB2340:EE_|SJ2PR10MB7014:EE_ X-MS-Office365-Filtering-Correlation-Id: 62d57827-362a-4415-4be4-08dad48e0b18 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: exA3Xj+gw6xfABMRXvzqSaviX/x30vHnVy4MYAbGPtyhd+exIY77Y2AQgQYPauLcelROlx6bC9QoNZQQf2PNqURZc6T4B06/ugfuFG2zGXiaz1D4F4F8P8ISDO1e9D/+qPeUO92hgoihbDW+d3BvNOgGEIBp4bGUJ0EcmUbNjN1EFPSu8m0Z7slkjK9WNMtRqp0JIBW0EqunwXy08aA2dG0a2njs1YJ5m1UDJT+FaXMS7ok6XA4edvMMJLqTzopCIDRCqLQvEUiwCcIjXrW7IxTRjlFOUnxMmskgMDg2xX2Fa1FW6he3xdKoEXQl4uDyWet/fvQ16bc/tfatpk2GQ3R/mBP8xYMuIvNrjMaCCNyM2JpCrurYrRJ6LjXoWg7wHEQhWwtKW4sEyUEmDH2tacDPlDuOk0feCquYwj4S1IgY7+cFARJupQSlNxvOSglXkIRF16pRBC7W2cLd7pGDwzQvpnvUA4Z+2o5LhWO/huDqTZQPYTU4BqiOf+1dVpGikOJd5IkWniLCvuB9Xby7eJVtvBsMhpbzlCwxpKuwgubSMxChBdJZePX8e3e0bze9t5u56SwwmugrZKACFAJVlPnUQ56+kLbQlZQ487IpDaKVLHiviVJeOox+qpqXOiFkVjoFkJX8shGzOQgfPD84MGLO8m1yxJDW9m/Eca2ZEaE= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BN6PR1001MB2340.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(396003)(346002)(376002)(136003)(39860400002)(366004)(451199015)(38100700002)(8676002)(66476007)(66556008)(66946007)(186003)(6512007)(316002)(4326008)(6916009)(44832011)(5660300002)(83380400001)(2906002)(36756003)(41300700001)(2616005)(1076003)(8936002)(478600001)(6506007)(84970400001)(6666004)(107886003)(6486002)(86362001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: P4Ba2BZReP/td8M0uyh2G5r0eLz99UAJbZEMM7np4nFjGwPZWx6DppH8O38euo5IaPJWnbBIwDDP8UUT5Fj+BH+mx/v37tsEfXPq0a66YgVtTGmM5hd7VQu6xg8AliSoB3QtqeUGISBzR+3Gw+TDWE5mQivyVVbgC2aqPokHZ4vFP0YjrN7GLuNwYGIA9/5YQ/6MMvGU4y/9uWKB3vjmV/HfQXpgyPOUnwjeOMA1Eo7nTjdiBUK42W+QL3AEwAcHuqFB0QgyV9z4FI1Zy/+J1uthtF/W786jvq5eI4YI+XWG4r75+6JEa1G1AeG+VtMbB6Tm98NNqaUk7N1E34Vrxl5KarMnbPQNRs//Z232MNBUwIVhdPE/Fao9/nd0Bt7NeDKp/691jmmmQv+jOafRE8Ey2iIG0I6b061xgXE8wKN08GNkBW14oZSbjxsEcN6xahMH0aYlrq5cWNctoz/sHsSK5kLikR+V8r8Illm+yeKi31GRbLlVH2Ie221a277+fym73/TcO+nxYYEaRA46xc1mi0l2dST0RGCr3YEiUBcKtkcYTtI7xyx5uT/rIp5pvQE9JZ3JfskTY0eeUVxrMSmhygfF1McWyI2f2yfFWMI9u2aDW5T3cbCDO8zLm8k4U6P6G/MlMGmZqZVwP1D6pNLAWr2O1XTjCqmyEEz+ihiPb2LPk1sBIPCMwkE1GmRmQkxBziCU2raZLm0YZqTPsGj3MnQxv6cs1++RIRP42cHLCWhSzcAMr2iHGFOoFK9/RtbqcmyBtNhHr2qT/d/jPyCQ3C67IgVPwsCZDsmJYiY9Womz/TvnJHJIr6/G36kB9lBRjgiIw2SG82QK8OABP+nZS0vg6khPkQMSG0vsip5cJoU9DfIrjFxIAu43ToWS1zcxRbqb2c6eO6xW69mg8hoIDg5pOQV5X6Y9njdeYh+sbnSunNsfUeGFdO0RqPehJrWOGNvK+/MXDSrs+vTCp1OZu+/0vV2Qp2WcCaBob79EKhY751lCYZ2osXHH+PUbaOYzJnidZOyn6pCWzzxGg/XuEQquIpKiT1OHLcOzp624BVFh/8okaZBZheVbNFL3iveooMWmtAoP8jALYc3gGWpJdC1G8QBVdsaORD4EZPWEG/kjEzz+zhZ2hDdjjL9+9Xe2DEQDwLJznwfSugYzKwi5dUO3JFbh+oSA1LdSvvLMb3QXsxUg7IorfrSR3Sfrnt9LKFOvrk6vCm0XeXYkAopm3f/a7mht9y6qvHlemTPGZg5eXF4QPhCkjGGRA5S2OIHTFgmvox9pUoPN3+uCPrccvgW2saXP5uN+eJORM+NKKbBZRV/DBPQqyMTksc10G+eI/RwnBTXBaPXFmRBU7Jy0IaRyeezJLPGKXuKVUJiKuk48lUmBk7TFOmYYUN7tc7+KbcCMgzMbgqfRsvx7K0MReKk6BTungSJcbBR61TuXJgPnpVLk5tLvixNH6UQLvVIkPJ/VTDMy3rIFcdOOarN1cVQixq2dJc72bmLWKOj8r+11yUf4ZVviAqmzhlAgRioZ14kUw8wnnv+jKWd3uaDhckYOHLVKnCVRbW6x5cL0ZEZ1yYcQTqJ7YHO+VVuUTn11VlMwmBq+j6HPl9Z3DfXeI2tHKKWIj4o4MWS/Prc= X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: JZRsfS9cdOpRu6rQLiyNJHetRCLoY9Tg5SFkoFzBta4GyFQOXR+IrcSpLWhmT4MlU6KQhJZtnMKUASAJX3tf1oSk6J3UT5jL40SKGWfh+C/iI3Xz2SQCCydpvo5wsknynaSo+B7iigdzMkfNMNf/b9J2PyW+idi0I/Euw0UhCk7/C1vX/X7MYrdr5GTod/FNqEHjIGZ5UuIM20S+7KC34ALBHdSymOvBfonGd8eNQlyQwYPfQQyRnzNokXox+20lt+0qRPsh2RAKvJHuI6VYm28FTED1VWKCChQvNsi95BWQeatlpNqshtZUwUjj18h63FsR8O/J7FGqameYZN2R812IukGFPBjPGkgCpQ3lG48N46iH2BnndVZ0GO+I3EyuYspcyrq9T1k8C+dRQTsDb7lTkk9MIH5OGhfCOWzDdBmo05eJkSq8fDVuHbi9tXf5eBpVAw1V8EVF+Vg8psEvfVvfA6kGY2drT5vQXauwID8we3WY4QIYTuYgk3WINOP8cq86FR3YIIbRlK+QzWNB0oZBkl/m/DMgxc905JFYLUVy6DFzz9HqRdBYua6Q4SBD95BRJchgCEqFnYthSlxdDhMlHSFf4p/bUce1x5ZvgbjNcFO41IDe0aSbgoD7O7/XvgRpOa48aNIxtEgd975duwjWXvelMwZjyj4Cxe6MJlTBUw+HKvtptlegnz26ORuXJlL8kh7HA9hGCTsSHM4M9mzMqEcePC2RSEoxJoMwIWFiJwvTfc2gYV47UKi5lTZc X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 62d57827-362a-4415-4be4-08dad48e0b18 X-MS-Exchange-CrossTenant-AuthSource: BN6PR1001MB2340.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Dec 2022 17:52:56.4651 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: qr9BNOxIyaU32teoHmRyJjaJRGLdJmO+Vl2/5PVTxIO6Qdv6ptmDvLFqf5K32OmCirYbhVQyAC0V0xL9v6iDNC2budKhfg2mYPPjC1rEAm4= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR10MB7014 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-12-02_10,2022-12-01_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 bulkscore=0 suspectscore=0 mlxlogscore=999 spamscore=0 phishscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2212020142 X-Proofpoint-ORIG-GUID: wgLFeurw3tlUkSqhitnWz19_-ydBNhBD X-Proofpoint-GUID: wgLFeurw3tlUkSqhitnWz19_-ydBNhBD X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Cupertino Miranda via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Cupertino Miranda <cupertino.miranda@oracle.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[1/2] select .rodata for const volatile variables.
|
|
Commit Message
Cupertino Miranda
Dec. 2, 2022, 5:52 p.m. UTC
This commit is a follow up of bugzilla #107181. The commit /a0aafbc/ changed the default implementation of the SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the placement of `const volatile' objects. However, the following targets use target-specific selection functions and they choke on the testcase pr25521.c: *rx - target sets its const variables as '.section C,"a",@progbits'. *powerpc - its 32bit version is eager to allocate globals in .sdata sections. Normally, one can expect for the variable to be allocated in .srodata, however, in case of powerpc-*-* or powerpc64-*-* (with -m32) 'targetm.have_srodata_section == false' and the code in categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. /* If the target uses small data sections, select it. */ else if (targetm.in_small_data_p (decl)) { if (ret == SECCAT_BSS) ret = SECCAT_SBSS; else if targetm.have_srodata_section && ret == SECCAT_RODATA) ret = SECCAT_SRODATA; else ret = SECCAT_SDATA; } LLVM compiler does not generate .sdata symbols at all, having different code generation even for non const volatile symbols. Targets that for acceptable reasons could not match the LLVM generated code were marked as XFAIL. gcc/testsuite/ChangeLog: * lib/target-supports.exp: Added check_effective_target_const_volatile_readonly_section. * gcc.dg/pr25521.c: Added XFAIL. --- gcc/testsuite/gcc.dg/pr25521.c | 3 +-- gcc/testsuite/lib/target-supports.exp | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-)
Comments
On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: > This commit is a follow up of bugzilla #107181. > > The commit /a0aafbc/ changed the default implementation of the > SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the > placement of `const volatile' objects. > > However, the following targets use target-specific selection functions > and they choke on the testcase pr25521.c: > > *rx - target sets its const variables as '.section C,"a",@progbits'. That's presumably a constant section. We should instead twiddle the test to recognize that section. > > *powerpc - its 32bit version is eager to allocate globals in .sdata > sections. > > Normally, one can expect for the variable to be allocated in .srodata, > however, in case of powerpc-*-* or powerpc64-*-* (with -m32) > 'targetm.have_srodata_section == false' and the code in > categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. > > /* If the target uses small data sections, select it. */ > else if (targetm.in_small_data_p (decl)) > { > if (ret == SECCAT_BSS) > ret = SECCAT_SBSS; > else if targetm.have_srodata_section && ret == SECCAT_RODATA) > ret = SECCAT_SRODATA; > else > ret = SECCAT_SDATA; > } I'd just skip the test for 32bit ppc. There should be suitable effective-target tests you can use. jeff
> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >> This commit is a follow up of bugzilla #107181. >> The commit /a0aafbc/ changed the default implementation of the >> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >> placement of `const volatile' objects. >> However, the following targets use target-specific selection functions >> and they choke on the testcase pr25521.c: >> *rx - target sets its const variables as '.section C,"a",@progbits'. > That's presumably a constant section. We should instead twiddle the test to > recognize that section. Although @progbits is indeed a constant section, I believe it is more interesting to detect if the `rx' starts selecting more standard sections instead of the current @progbits. That was the reason why I opted to XFAIL instead of PASSing it. Can I keep it as such ? > >> *powerpc - its 32bit version is eager to allocate globals in .sdata >> sections. >> Normally, one can expect for the variable to be allocated in .srodata, >> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >> 'targetm.have_srodata_section == false' and the code in >> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >> /* If the target uses small data sections, select it. */ >> else if (targetm.in_small_data_p (decl)) >> { >> if (ret == SECCAT_BSS) >> ret = SECCAT_SBSS; >> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >> ret = SECCAT_SRODATA; >> else >> ret = SECCAT_SDATA; >> } > I'd just skip the test for 32bit ppc. There should be suitable effective-target > tests you can use. > > jeff
gentle ping Cupertino Miranda writes: >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> This commit is a follow up of bugzilla #107181. >>> The commit /a0aafbc/ changed the default implementation of the >>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>> placement of `const volatile' objects. >>> However, the following targets use target-specific selection functions >>> and they choke on the testcase pr25521.c: >>> *rx - target sets its const variables as '.section C,"a",@progbits'. >> That's presumably a constant section. We should instead twiddle the test to >> recognize that section. > > Although @progbits is indeed a constant section, I believe it is > more interesting to detect if the `rx' starts selecting more > standard sections instead of the current @progbits. > That was the reason why I opted to XFAIL instead of PASSing it. > Can I keep it as such ? > >> >>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>> sections. >>> Normally, one can expect for the variable to be allocated in .srodata, >>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>> 'targetm.have_srodata_section == false' and the code in >>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>> /* If the target uses small data sections, select it. */ >>> else if (targetm.in_small_data_p (decl)) >>> { >>> if (ret == SECCAT_BSS) >>> ret = SECCAT_SBSS; >>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>> ret = SECCAT_SRODATA; >>> else >>> ret = SECCAT_SDATA; >>> } >> I'd just skip the test for 32bit ppc. There should be suitable effective-target >> tests you can use. >> >> jeff
Cupertino Miranda via Gcc-patches writes: > gentle ping > > Cupertino Miranda writes: > >>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>> This commit is a follow up of bugzilla #107181. >>>> The commit /a0aafbc/ changed the default implementation of the >>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>> placement of `const volatile' objects. >>>> However, the following targets use target-specific selection functions >>>> and they choke on the testcase pr25521.c: >>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>> That's presumably a constant section. We should instead twiddle the test to >>> recognize that section. >> >> Although @progbits is indeed a constant section, I believe it is >> more interesting to detect if the `rx' starts selecting more >> standard sections instead of the current @progbits. >> That was the reason why I opted to XFAIL instead of PASSing it. >> Can I keep it as such ? >> >>> >>>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>>> sections. >>>> Normally, one can expect for the variable to be allocated in .srodata, >>>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>>> 'targetm.have_srodata_section == false' and the code in >>>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>>> /* If the target uses small data sections, select it. */ >>>> else if (targetm.in_small_data_p (decl)) >>>> { >>>> if (ret == SECCAT_BSS) >>>> ret = SECCAT_SBSS; >>>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>>> ret = SECCAT_SRODATA; >>>> else >>>> ret = SECCAT_SDATA; >>>> } >>> I'd just skip the test for 32bit ppc. There should be suitable effective-target >>> tests you can use. >>> >>> jeff
PING PING Cupertino Miranda writes: > Cupertino Miranda via Gcc-patches writes: > >> gentle ping >> >> Cupertino Miranda writes: >> >>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>> This commit is a follow up of bugzilla #107181. >>>>> The commit /a0aafbc/ changed the default implementation of the >>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>> placement of `const volatile' objects. >>>>> However, the following targets use target-specific selection functions >>>>> and they choke on the testcase pr25521.c: >>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>> That's presumably a constant section. We should instead twiddle the test to >>>> recognize that section. >>> >>> Although @progbits is indeed a constant section, I believe it is >>> more interesting to detect if the `rx' starts selecting more >>> standard sections instead of the current @progbits. >>> That was the reason why I opted to XFAIL instead of PASSing it. >>> Can I keep it as such ? >>> >>>> >>>>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>>>> sections. >>>>> Normally, one can expect for the variable to be allocated in .srodata, >>>>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>>>> 'targetm.have_srodata_section == false' and the code in >>>>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>>>> /* If the target uses small data sections, select it. */ >>>>> else if (targetm.in_small_data_p (decl)) >>>>> { >>>>> if (ret == SECCAT_BSS) >>>>> ret = SECCAT_SBSS; >>>>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>>>> ret = SECCAT_SRODATA; >>>>> else >>>>> ret = SECCAT_SDATA; >>>>> } >>>> I'd just skip the test for 32bit ppc. There should be suitable effective-target >>>> tests you can use. >>>> >>>> jeff
Cupertino Miranda writes: >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> This commit is a follow up of bugzilla #107181. >>> The commit /a0aafbc/ changed the default implementation of the >>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>> placement of `const volatile' objects. >>> However, the following targets use target-specific selection functions >>> and they choke on the testcase pr25521.c: >>> *rx - target sets its const variables as '.section C,"a",@progbits'. >> That's presumably a constant section. We should instead twiddle the test to >> recognize that section. > > Although @progbits is indeed a constant section, I believe it is > more interesting to detect if the `rx' starts selecting more > standard sections instead of the current @progbits. > That was the reason why I opted to XFAIL instead of PASSing it. > Can I keep it as such ? > Jeff: Can you please give me an answer on this ? Cupertino >> >>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>> sections. >>> Normally, one can expect for the variable to be allocated in .srodata, >>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>> 'targetm.have_srodata_section == false' and the code in >>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>> /* If the target uses small data sections, select it. */ >>> else if (targetm.in_small_data_p (decl)) >>> { >>> if (ret == SECCAT_BSS) >>> ret = SECCAT_SBSS; >>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>> ret = SECCAT_SRODATA; >>> else >>> ret = SECCAT_SDATA; >>> } >> I'd just skip the test for 32bit ppc. There should be suitable effective-target >> tests you can use. >> >> jeff
On 12/7/22 08:45, Cupertino Miranda wrote: > >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> This commit is a follow up of bugzilla #107181. >>> The commit /a0aafbc/ changed the default implementation of the >>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>> placement of `const volatile' objects. >>> However, the following targets use target-specific selection functions >>> and they choke on the testcase pr25521.c: >>> *rx - target sets its const variables as '.section C,"a",@progbits'. >> That's presumably a constant section. We should instead twiddle the test to >> recognize that section. > > Although @progbits is indeed a constant section, I believe it is > more interesting to detect if the `rx' starts selecting more > standard sections instead of the current @progbits. > That was the reason why I opted to XFAIL instead of PASSing it. > Can I keep it as such ? I'm not aware of any ongoing development for that port, so I would not let concerns about the rx port changing behavior dominate how we approach this problem. The rx port is using a different name for the section. That's valid thing to do and to the extent we can, we should support that in the test rather than (incorrectly IMHO) xfailing the test just becuase the name isn't what we expected. To avoid over-eagerly matching, I would probably search for "C," I wouldn't do that for the const or rodata sections as they often have a suffix like 1, 2, 4, 8 for different sized rodata sections. PPC32 is explicitly doing something different and placing those objects into an RW section. So for PPC32 it makes more sense to skip the test rather than xfail it. Jeff
Thank you for the comments and suggestions. I have changed the patch. Unfortunately in case of rx target I could not make scan-assembler-symbol-section to match. I believe it is because the .section and .global entries order is reversed in this target. Patch in inlined below. looking forward to your comments. Cupertino Jeff Law writes: > On 12/7/22 08:45, Cupertino Miranda wrote: >> >>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>> This commit is a follow up of bugzilla #107181. >>>> The commit /a0aafbc/ changed the default implementation of the >>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>> placement of `const volatile' objects. >>>> However, the following targets use target-specific selection functions >>>> and they choke on the testcase pr25521.c: >>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>> That's presumably a constant section. We should instead twiddle the test to >>> recognize that section. >> Although @progbits is indeed a constant section, I believe it is >> more interesting to detect if the `rx' starts selecting more >> standard sections instead of the current @progbits. >> That was the reason why I opted to XFAIL instead of PASSing it. >> Can I keep it as such ? > I'm not aware of any ongoing development for that port, so I would not let > concerns about the rx port changing behavior dominate how we approach this > problem. > > The rx port is using a different name for the section. That's valid thing to > do and to the extent we can, we should support that in the test rather than > (incorrectly IMHO) xfailing the test just becuase the name isn't what we > expected. > > To avoid over-eagerly matching, I would probably search for "C," I wouldn't do > that for the const or rodata sections as they often have a suffix like 1, 2, 4, > 8 for different sized rodata sections. > > PPC32 is explicitly doing something different and placing those objects into an > RW section. So for PPC32 it makes more sense to skip the test rather than xfail > it. > > Jeff
Cupertino Miranda via Gcc-patches writes: > Thank you for the comments and suggestions. > I have changed the patch. > > Unfortunately in case of rx target I could not make > scan-assembler-symbol-section to match. I believe it is because the > .section and .global entries order is reversed in this target. > > Patch in inlined below. looking forward to your comments. > > Cupertino > > diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c > index 63363a03b9f..82b4cd88ec0 100644 > --- a/gcc/testsuite/gcc.dg/pr25521.c > +++ b/gcc/testsuite/gcc.dg/pr25521.c > @@ -2,9 +2,10 @@ > sections. > > { dg-require-effective-target elf } > - { dg-do compile } */ > + { dg-do compile } > + { dg-skip-if "" { ! const_volatile_readonly_section } } */ > > const volatile int foo = 30; > > - > -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ > +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ > +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index c0694af2338..91aafd89909 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { > > return 1 > } > + > +# returns 1 if target does selects a readonly section for const volatile variables. > +proc check_effective_target_const_volatile_readonly_section { } { > + > + if { [istarget powerpc-*-*] > + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { > + return 0 > + } > + return 1 > +} > > > Jeff Law writes: > >> On 12/7/22 08:45, Cupertino Miranda wrote: >>> >>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>> This commit is a follow up of bugzilla #107181. >>>>> The commit /a0aafbc/ changed the default implementation of the >>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>> placement of `const volatile' objects. >>>>> However, the following targets use target-specific selection functions >>>>> and they choke on the testcase pr25521.c: >>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>> That's presumably a constant section. We should instead twiddle the test to >>>> recognize that section. >>> Although @progbits is indeed a constant section, I believe it is >>> more interesting to detect if the `rx' starts selecting more >>> standard sections instead of the current @progbits. >>> That was the reason why I opted to XFAIL instead of PASSing it. >>> Can I keep it as such ? >> I'm not aware of any ongoing development for that port, so I would not let >> concerns about the rx port changing behavior dominate how we approach this >> problem. >> >> The rx port is using a different name for the section. That's valid thing to >> do and to the extent we can, we should support that in the test rather than >> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >> expected. >> >> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >> 8 for different sized rodata sections. >> >> PPC32 is explicitly doing something different and placing those objects into an >> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >> it. >> >> Jeff
Hi Jeff, Can you please confirm if the patch is Ok? Thanks, Cupertino > Cupertino Miranda via Gcc-patches writes: > >> Thank you for the comments and suggestions. >> I have changed the patch. >> >> Unfortunately in case of rx target I could not make >> scan-assembler-symbol-section to match. I believe it is because the >> .section and .global entries order is reversed in this target. >> >> Patch in inlined below. looking forward to your comments. >> >> Cupertino >> >> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >> index 63363a03b9f..82b4cd88ec0 100644 >> --- a/gcc/testsuite/gcc.dg/pr25521.c >> +++ b/gcc/testsuite/gcc.dg/pr25521.c >> @@ -2,9 +2,10 @@ >> sections. >> >> { dg-require-effective-target elf } >> - { dg-do compile } */ >> + { dg-do compile } >> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >> >> const volatile int foo = 30; >> >> - >> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >> index c0694af2338..91aafd89909 100644 >> --- a/gcc/testsuite/lib/target-supports.exp >> +++ b/gcc/testsuite/lib/target-supports.exp >> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >> >> return 1 >> } >> + >> +# returns 1 if target does selects a readonly section for const volatile variables. >> +proc check_effective_target_const_volatile_readonly_section { } { >> + >> + if { [istarget powerpc-*-*] >> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >> + return 0 >> + } >> + return 1 >> +} >> >> >> Jeff Law writes: >> >>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>> >>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>> This commit is a follow up of bugzilla #107181. >>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>> placement of `const volatile' objects. >>>>>> However, the following targets use target-specific selection functions >>>>>> and they choke on the testcase pr25521.c: >>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>> That's presumably a constant section. We should instead twiddle the test to >>>>> recognize that section. >>>> Although @progbits is indeed a constant section, I believe it is >>>> more interesting to detect if the `rx' starts selecting more >>>> standard sections instead of the current @progbits. >>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>> Can I keep it as such ? >>> I'm not aware of any ongoing development for that port, so I would not let >>> concerns about the rx port changing behavior dominate how we approach this >>> problem. >>> >>> The rx port is using a different name for the section. That's valid thing to >>> do and to the extent we can, we should support that in the test rather than >>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>> expected. >>> >>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>> 8 for different sized rodata sections. >>> >>> PPC32 is explicitly doing something different and placing those objects into an >>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>> it. >>> >>> Jeff
PING !!!!! Cupertino Miranda via Gcc-patches writes: > Hi Jeff, > > Can you please confirm if the patch is Ok? > > Thanks, > Cupertino > >> Cupertino Miranda via Gcc-patches writes: >> >>> Thank you for the comments and suggestions. >>> I have changed the patch. >>> >>> Unfortunately in case of rx target I could not make >>> scan-assembler-symbol-section to match. I believe it is because the >>> .section and .global entries order is reversed in this target. >>> >>> Patch in inlined below. looking forward to your comments. >>> >>> Cupertino >>> >>> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >>> index 63363a03b9f..82b4cd88ec0 100644 >>> --- a/gcc/testsuite/gcc.dg/pr25521.c >>> +++ b/gcc/testsuite/gcc.dg/pr25521.c >>> @@ -2,9 +2,10 @@ >>> sections. >>> >>> { dg-require-effective-target elf } >>> - { dg-do compile } */ >>> + { dg-do compile } >>> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >>> >>> const volatile int foo = 30; >>> >>> - >>> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >>> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >>> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >>> index c0694af2338..91aafd89909 100644 >>> --- a/gcc/testsuite/lib/target-supports.exp >>> +++ b/gcc/testsuite/lib/target-supports.exp >>> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >>> >>> return 1 >>> } >>> + >>> +# returns 1 if target does selects a readonly section for const volatile variables. >>> +proc check_effective_target_const_volatile_readonly_section { } { >>> + >>> + if { [istarget powerpc-*-*] >>> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >>> + return 0 >>> + } >>> + return 1 >>> +} >>> >>> >>> Jeff Law writes: >>> >>>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>>> >>>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>>> This commit is a follow up of bugzilla #107181. >>>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>>> placement of `const volatile' objects. >>>>>>> However, the following targets use target-specific selection functions >>>>>>> and they choke on the testcase pr25521.c: >>>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>>> That's presumably a constant section. We should instead twiddle the test to >>>>>> recognize that section. >>>>> Although @progbits is indeed a constant section, I believe it is >>>>> more interesting to detect if the `rx' starts selecting more >>>>> standard sections instead of the current @progbits. >>>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>>> Can I keep it as such ? >>>> I'm not aware of any ongoing development for that port, so I would not let >>>> concerns about the rx port changing behavior dominate how we approach this >>>> problem. >>>> >>>> The rx port is using a different name for the section. That's valid thing to >>>> do and to the extent we can, we should support that in the test rather than >>>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>>> expected. >>>> >>>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>>> 8 for different sized rodata sections. >>>> >>>> PPC32 is explicitly doing something different and placing those objects into an >>>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>>> it. >>>> >>>> Jeff
Hi Jeff, Please, please, give me some feedback on this one. I just don't want to have to keep asking you for time on this small pending patches that I also have to keep track on. I realized your committed the other one. Thank you ! Best regards, Cupertino Cupertino Miranda writes: > PING !!!!! > > Cupertino Miranda via Gcc-patches writes: > >> Hi Jeff, >> >> Can you please confirm if the patch is Ok? >> >> Thanks, >> Cupertino >> >>> Cupertino Miranda via Gcc-patches writes: >>> >>>> Thank you for the comments and suggestions. >>>> I have changed the patch. >>>> >>>> Unfortunately in case of rx target I could not make >>>> scan-assembler-symbol-section to match. I believe it is because the >>>> .section and .global entries order is reversed in this target. >>>> >>>> Patch in inlined below. looking forward to your comments. >>>> >>>> Cupertino >>>> >>>> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >>>> index 63363a03b9f..82b4cd88ec0 100644 >>>> --- a/gcc/testsuite/gcc.dg/pr25521.c >>>> +++ b/gcc/testsuite/gcc.dg/pr25521.c >>>> @@ -2,9 +2,10 @@ >>>> sections. >>>> >>>> { dg-require-effective-target elf } >>>> - { dg-do compile } */ >>>> + { dg-do compile } >>>> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >>>> >>>> const volatile int foo = 30; >>>> >>>> - >>>> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >>>> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >>>> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >>>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >>>> index c0694af2338..91aafd89909 100644 >>>> --- a/gcc/testsuite/lib/target-supports.exp >>>> +++ b/gcc/testsuite/lib/target-supports.exp >>>> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >>>> >>>> return 1 >>>> } >>>> + >>>> +# returns 1 if target does selects a readonly section for const volatile variables. >>>> +proc check_effective_target_const_volatile_readonly_section { } { >>>> + >>>> + if { [istarget powerpc-*-*] >>>> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >>>> + return 0 >>>> + } >>>> + return 1 >>>> +} >>>> >>>> >>>> Jeff Law writes: >>>> >>>>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>>>> >>>>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>>>> This commit is a follow up of bugzilla #107181. >>>>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>>>> placement of `const volatile' objects. >>>>>>>> However, the following targets use target-specific selection functions >>>>>>>> and they choke on the testcase pr25521.c: >>>>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>>>> That's presumably a constant section. We should instead twiddle the test to >>>>>>> recognize that section. >>>>>> Although @progbits is indeed a constant section, I believe it is >>>>>> more interesting to detect if the `rx' starts selecting more >>>>>> standard sections instead of the current @progbits. >>>>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>>>> Can I keep it as such ? >>>>> I'm not aware of any ongoing development for that port, so I would not let >>>>> concerns about the rx port changing behavior dominate how we approach this >>>>> problem. >>>>> >>>>> The rx port is using a different name for the section. That's valid thing to >>>>> do and to the extent we can, we should support that in the test rather than >>>>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>>>> expected. >>>>> >>>>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>>>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>>>> 8 for different sized rodata sections. >>>>> >>>>> PPC32 is explicitly doing something different and placing those objects into an >>>>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>>>> it. >>>>> >>>>> Jeff
[PING] Cupertino Miranda writes: > Hi Jeff, > > Please, please, give me some feedback on this one. > I just don't want to have to keep asking you for time on this small > pending patches that I also have to keep track on. > > I realized your committed the other one. Thank you ! > > Best regards, > Cupertino > > > Cupertino Miranda writes: > >> PING !!!!! >> >> Cupertino Miranda via Gcc-patches writes: >> >>> Hi Jeff, >>> >>> Can you please confirm if the patch is Ok? >>> >>> Thanks, >>> Cupertino >>> >>>> Cupertino Miranda via Gcc-patches writes: >>>> >>>>> Thank you for the comments and suggestions. >>>>> I have changed the patch. >>>>> >>>>> Unfortunately in case of rx target I could not make >>>>> scan-assembler-symbol-section to match. I believe it is because the >>>>> .section and .global entries order is reversed in this target. >>>>> >>>>> Patch in inlined below. looking forward to your comments. >>>>> >>>>> Cupertino >>>>> >>>>> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >>>>> index 63363a03b9f..82b4cd88ec0 100644 >>>>> --- a/gcc/testsuite/gcc.dg/pr25521.c >>>>> +++ b/gcc/testsuite/gcc.dg/pr25521.c >>>>> @@ -2,9 +2,10 @@ >>>>> sections. >>>>> >>>>> { dg-require-effective-target elf } >>>>> - { dg-do compile } */ >>>>> + { dg-do compile } >>>>> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >>>>> >>>>> const volatile int foo = 30; >>>>> >>>>> - >>>>> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >>>>> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >>>>> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >>>>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >>>>> index c0694af2338..91aafd89909 100644 >>>>> --- a/gcc/testsuite/lib/target-supports.exp >>>>> +++ b/gcc/testsuite/lib/target-supports.exp >>>>> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >>>>> >>>>> return 1 >>>>> } >>>>> + >>>>> +# returns 1 if target does selects a readonly section for const volatile variables. >>>>> +proc check_effective_target_const_volatile_readonly_section { } { >>>>> + >>>>> + if { [istarget powerpc-*-*] >>>>> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >>>>> + return 0 >>>>> + } >>>>> + return 1 >>>>> +} >>>>> >>>>> >>>>> Jeff Law writes: >>>>> >>>>>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>>>>> >>>>>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>>>>> This commit is a follow up of bugzilla #107181. >>>>>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>>>>> placement of `const volatile' objects. >>>>>>>>> However, the following targets use target-specific selection functions >>>>>>>>> and they choke on the testcase pr25521.c: >>>>>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>>>>> That's presumably a constant section. We should instead twiddle the test to >>>>>>>> recognize that section. >>>>>>> Although @progbits is indeed a constant section, I believe it is >>>>>>> more interesting to detect if the `rx' starts selecting more >>>>>>> standard sections instead of the current @progbits. >>>>>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>>>>> Can I keep it as such ? >>>>>> I'm not aware of any ongoing development for that port, so I would not let >>>>>> concerns about the rx port changing behavior dominate how we approach this >>>>>> problem. >>>>>> >>>>>> The rx port is using a different name for the section. That's valid thing to >>>>>> do and to the extent we can, we should support that in the test rather than >>>>>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>>>>> expected. >>>>>> >>>>>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>>>>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>>>>> 8 for different sized rodata sections. >>>>>> >>>>>> PPC32 is explicitly doing something different and placing those objects into an >>>>>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>>>>> it. >>>>>> >>>>>> Jeff
On 1/24/23 05:24, Cupertino Miranda wrote: > Thank you for the comments and suggestions. > I have changed the patch. > > Unfortunately in case of rx target I could not make > scan-assembler-symbol-section to match. I believe it is because the > .section and .global entries order is reversed in this target. > > Patch in inlined below. looking forward to your comments. Sorry for the long delays. I've installed this version. As a follow-up, can you update the documentation in doc/sourcebuild.texi to include the new check-effective-target test? Thanks, Jeff
> On 1/24/23 05:24, Cupertino Miranda wrote: >> Thank you for the comments and suggestions. >> I have changed the patch. >> Unfortunately in case of rx target I could not make >> scan-assembler-symbol-section to match. I believe it is because the >> .section and .global entries order is reversed in this target. >> Patch in inlined below. looking forward to your comments. > Sorry for the long delays. I've installed this version. > > As a follow-up, can you update the documentation in doc/sourcebuild.texi to > include the new check-effective-target test? Hi Jeff, Thank you for installing the patch. I have prepared the doc change you requested. Hopefully this is what you were expecting. Regards, Cupertino
Cupertino Miranda via Gcc-patches writes: >> On 1/24/23 05:24, Cupertino Miranda wrote: >>> Thank you for the comments and suggestions. >>> I have changed the patch. >>> Unfortunately in case of rx target I could not make >>> scan-assembler-symbol-section to match. I believe it is because the >>> .section and .global entries order is reversed in this target. >>> Patch in inlined below. looking forward to your comments. >> Sorry for the long delays. I've installed this version. >> >> As a follow-up, can you update the documentation in doc/sourcebuild.texi to >> include the new check-effective-target test? > > Hi Jeff, > > Thank you for installing the patch. > I have prepared the doc change you requested. > Hopefully this is what you were expecting. > > Regards, > Cupertino Just realized previous patch was in incorrect placement alphabetically. Please consider this one instead. Cupertino
On 3/13/23 11:57, Cupertino Miranda wrote: > > Cupertino Miranda via Gcc-patches writes: > >>> On 1/24/23 05:24, Cupertino Miranda wrote: >>>> Thank you for the comments and suggestions. >>>> I have changed the patch. >>>> Unfortunately in case of rx target I could not make >>>> scan-assembler-symbol-section to match. I believe it is because the >>>> .section and .global entries order is reversed in this target. >>>> Patch in inlined below. looking forward to your comments. >>> Sorry for the long delays. I've installed this version. >>> >>> As a follow-up, can you update the documentation in doc/sourcebuild.texi to >>> include the new check-effective-target test? >> >> Hi Jeff, >> >> Thank you for installing the patch. >> I have prepared the doc change you requested. >> Hopefully this is what you were expecting. >> >> Regards, >> Cupertino > > Just realized previous patch was in incorrect placement alphabetically. > Please consider this one instead. Installed. Thanks. jeff
diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c index 63363a03b9f..597a2fc25d8 100644 --- a/gcc/testsuite/gcc.dg/pr25521.c +++ b/gcc/testsuite/gcc.dg/pr25521.c @@ -6,5 +6,4 @@ const volatile int foo = 30; - -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { xfail { ! const_volatile_readonly_section } } } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 2a058c67c53..631d4593447 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -12196,3 +12196,15 @@ proc check_is_prog_name_available { prog } { return 1 } + +# returns 1 if target does selects a readonly section for const volatile variables. +proc check_effective_target_const_volatile_readonly_section { } { + + if { [istarget rx*-*-*] + || [istarget powerpc-*-*] + || [istarget rs6000*-*-*] + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { + return 0 + } + return 1 +}