From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 9D6283858C56; Tue, 26 Jul 2022 21:37:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9D6283858C56 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 26QLaOEb021749; Tue, 26 Jul 2022 16:36:24 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 26QLaNtt021747; Tue, 26 Jul 2022 16:36:23 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Tue, 26 Jul 2022 16:36:22 -0500 From: Segher Boessenkool To: Jiufu Guo Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, linkw@gcc.gnu.org Subject: Re: [PATCH V1] HIGH part of symbol ref is invalid for constant pool Message-ID: <20220726213621.GB25951@gate.crashing.org> References: <20220719143054.82832-1-guojiufu@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220719143054.82832-1-guojiufu@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jul 2022 21:37:27 -0000 Hi! On Tue, Jul 19, 2022 at 10:30:54PM +0800, Jiufu Guo wrote: > In patch https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597712.html, > test case was not added. After more check, a testcase is added for it. > > The high part of the symbol address is invalid for the constant pool. Invalid, how so? Is there a PR related here? But it is not particularly useful ever, either: we do not know two different addresses will have the same HIGH unless we know the exact address, and then we don't need HIGH anyway. > * config/rs6000/rs6000.cc (rs6000_cannot_force_const_mem): > Return true for HIGH code rtx. * config/rs6000/rs6000.cc (rs6000_cannot_force_const_mem): Return true for HIGH code rtx. Please don't wrap lines early: changelog lines are 80 positions long, including the leading tab (which counts as eight positions). > static bool > rs6000_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, rtx x) > { > - if (GET_CODE (x) == HIGH > - && GET_CODE (XEXP (x, 0)) == UNSPEC) > + /* High part of a symbol ref/address can not be put into constant pool. e.g. > + (high:DI (symbol_ref:DI ("var")..)) or > + (high:DI (unspec:DI [(symbol_ref/u:DI ("*.LC0")..) > + (high:DI (const:DI (plus:DI (symbol_ref:DI ("xx")) (const_int 12)))). */ > + if (GET_CODE (x) == HIGH) > return true; I'm not sure the new comment is helpful at all? Are these examples of where the compiler (or assembler perhaps) will choke? > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/constpoolcheck.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile { target powerpc*-*-* } } */ Everything in gcc.target/powerpc is target powerpc* always. > +/* { dg-options "-O1 -mdejagnu-cpu=power10" } */ > +/* (high:DI (symbol_ref:DI ("var_48")..))) should not cause ICE. */ Ah, so there is an ICE, I see. Please open a PR, and mention that in the testcase as well as in the commit message and changelog. I agree with what the patch does, it just needs a little more work :-) Segher