Message ID | ccb5f603-5c1f-0e59-31d9-07251f81e0aa@linux.ibm.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 81BB73858012 for <patchwork@sourceware.org>; Mon, 29 Nov 2021 21:57:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 81BB73858012 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1638223061; bh=0UNtvJOEkBhI6Dt8meHsXAssOYdsdcgaDNONfuIymWo=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=LaYid1piTVjyenvyKqVjzzNUs6qNThmRAQswfiM8WKINffESRHU2k+yX7+xjpWbyH NUtcRhsy5CXcMQPjewjQQwsQSEDLJ6U3GYmF6Fn53+ZTGVv5RM7oem9ZFe82Swhxve WWeXmF2GeFetQ3BGP51fjt4Pqh00i+ipXrnGT1jA= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id E9EAE385841C for <gcc-patches@gcc.gnu.org>; Mon, 29 Nov 2021 21:57:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E9EAE385841C Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1ATLGF66019726; Mon, 29 Nov 2021 21:57:11 GMT Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 3cn6mq8s7x-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 29 Nov 2021 21:57:10 +0000 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1ATLlUk7016093; Mon, 29 Nov 2021 21:57:09 GMT Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by ppma04wdc.us.ibm.com with ESMTP id 3ckcaaw9ru-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 29 Nov 2021 21:57:09 +0000 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1ATLv9Pb31195518 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 29 Nov 2021 21:57:09 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id BBD961240A9; Mon, 29 Nov 2021 21:57:06 +0000 (GMT) Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 64AE112409A; Mon, 29 Nov 2021 21:57:06 +0000 (GMT) Received: from [9.160.47.23] (unknown [9.160.47.23]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTP; Mon, 29 Nov 2021 21:57:06 +0000 (GMT) Message-ID: <ccb5f603-5c1f-0e59-31d9-07251f81e0aa@linux.ibm.com> Date: Mon, 29 Nov 2021 15:56:58 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.3.2 Content-Language: en-US To: GCC Patches <gcc-patches@gcc.gnu.org> Subject: [PATCH] middle-end: Skip initialization of opaque type register variables [PR103127] Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: 4Ju_uvagqum26VAl_vk-N9hHc71WXpBL X-Proofpoint-ORIG-GUID: 4Ju_uvagqum26VAl_vk-N9hHc71WXpBL X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475 definitions=2021-11-29_11,2021-11-28_01,2020-04-07_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 adultscore=0 impostorscore=0 mlxscore=0 mlxlogscore=999 lowpriorityscore=0 priorityscore=1501 clxscore=1011 suspectscore=0 bulkscore=0 spamscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2111290100 X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Peter Bergner via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Peter Bergner <bergner@linux.ibm.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 |
middle-end: Skip initialization of opaque type register variables [PR103127]
|
|
Commit Message
Peter Bergner
Nov. 29, 2021, 9:56 p.m. UTC
Sorry for dropping the ball on testing the patch from the bugzilla! The following patch fixes the ICE reported in the bugzilla on the pre-existing gcc testsuite test case, bootstraps and shows no testsuite regressions on powerpc64le-linux. Ok for trunk? Peter For -ftrivial-auto-var-init=*, skip initializing the register variable if it is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes. gcc/ PR middle-end/103127 * internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque.
Comments
Peter, Thanks a lot for the patch. Richard, how do you think of the patch? (The major concern for me is: With the current patch proposed by Peter, we will generate the call to .DEFERRED_INIT for a variable with OPAQUE_TYPE during gimplification phase, However, if this variable is in register, then the call to .DEFERRED_INIT will NOT be expanded during RTL expansion phase. This unexpanded call to .DEFERRED_INIT might cause some potential IR issue later? If the above is a real issue, should we skip initialization for all OPAQUE_TYPE variables even when they are in memory and can be initialized with memset? then we should update “is_var_need_auto_init” in gimplify.c instead. However, the issue with this approach is, we might miss the opportunity to initialize an OPAQUE_TYPE variable if it will be in memory? ). Thanks. Qing > On Nov 29, 2021, at 3:56 PM, Peter Bergner <bergner@linux.ibm.com> wrote: > > Sorry for dropping the ball on testing the patch from the bugzilla! > > The following patch fixes the ICE reported in the bugzilla on the pre-existing > gcc testsuite test case, bootstraps and shows no testsuite regressions > on powerpc64le-linux. Ok for trunk? > > Peter > > > For -ftrivial-auto-var-init=*, skip initializing the register variable if it > is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes. > > gcc/ > PR middle-end/103127 > * internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque. > > diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c > index 0cba95411a6..7cc0e9d5293 100644 > --- a/gcc/internal-fn.c > +++ b/gcc/internal-fn.c > @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt) > } > else > { > + /* Skip variables of opaque types that are in a register. */ > + if (OPAQUE_TYPE_P (var_type)) > + return; > + > /* If this variable is in a register use expand_assignment. > For boolean scalars force zero-init. */ > tree init;
On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote: > > Peter, > > Thanks a lot for the patch. > > Richard, how do you think of the patch? > > (The major concern for me is: > > With the current patch proposed by Peter, we will generate the call to .DEFERRED_INIT for a variable with OPAQUE_TYPE during gimplification phase, > However, if this variable is in register, then the call to .DEFERRED_INIT will NOT be expanded during RTL expansion phase. This unexpanded call to .DEFERRED_INIT might cause some potential IR issue later? I think that's inconsistent indeed. Peter, what are "opaque" registers? rs6000-modes.def suggests that there's __vector_pair and __vector_quad, what's the GIMPLE types for those? It seems they are either SSA names or expanded to pseudo registers but there's no constants for them. > > If the above is a real issue, should we skip initialization for all OPAQUE_TYPE variables even when they are in memory and can be initialized with memset? > then we should update “is_var_need_auto_init” in gimplify.c instead. However, the issue with this approach is, we might miss the opportunity to initialize an OPAQUE_TYPE variable if it will be in memory? > ). I think we need to bite the bullet at some point to do register initialization not via expand_assignment but directly based on what the LHS expands to. Can they be initialized? I see they can be copied at least. If such "things" cannot be initialized they should indeed be exempt from auto-init. The documentation suggests that they act as bit-bucked but even bit-buckets should be initializable, thus why exactly does CONST0_RTX not exist for them? Richard. > > Thanks. > > Qing > > > > On Nov 29, 2021, at 3:56 PM, Peter Bergner <bergner@linux.ibm.com> wrote: > > > > Sorry for dropping the ball on testing the patch from the bugzilla! > > > > The following patch fixes the ICE reported in the bugzilla on the pre-existing > > gcc testsuite test case, bootstraps and shows no testsuite regressions > > on powerpc64le-linux. Ok for trunk? > > > > Peter > > > > > > For -ftrivial-auto-var-init=*, skip initializing the register variable if it > > is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes. > > > > gcc/ > > PR middle-end/103127 > > * internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque. > > > > diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c > > index 0cba95411a6..7cc0e9d5293 100644 > > --- a/gcc/internal-fn.c > > +++ b/gcc/internal-fn.c > > @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt) > > } > > else > > { > > + /* Skip variables of opaque types that are in a register. */ > > + if (OPAQUE_TYPE_P (var_type)) > > + return; > > + > > /* If this variable is in a register use expand_assignment. > > For boolean scalars force zero-init. */ > > tree init; >
On 11/30/21 2:37 AM, Richard Biener wrote: > On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote: > I think that's inconsistent indeed. Peter, what are "opaque" > registers? rs6000-modes.def suggests > that there's __vector_pair and __vector_quad, what's the GIMPLE types > for those? It seems they > are either SSA names or expanded to pseudo registers but there's no > constants for them. The __vector_pair and __vector_quad types are target specific types for use with our Matrix-Math-Assist (MMA) unit and they are only usable with our associated MMA built-in functions. What they hold is really dependent on which MMA built-ins you use on them. You can think of them a generic (and large) vector type where the subtype is undefined...or defined by which built-in function you happen to be using. We do not have any constants defined for them. How we initialize them is either by loading values from memory into them or by zeroing them out using the xxsetaccz instruction (only for __vector_quads). > Can they be initialized? I see they can be copied at least. __vector_quads can be zero initialized using the __builtin_mma_xxsetaccz() built-in function. We don't have a method (or use case) for zero initializing __vector_pairs. > If such "things" cannot be initialized they should indeed be exempt > from auto-init. The > documentation suggests that they act as bit-bucked but even bit-buckets should > be initializable, thus why exactly does CONST0_RTX not exist for them? We used to have CONST0_RTX defined (but nothing else), but we had problems with the compiler CSEing the initialization for multiple __vector_quads and then copying the values around. We'd end up with one xxsetaccz instruction and copies out of that accumulator register into the other accumulator registers. Copies are VERY expensive, while xxsetaccz's are cheap, so we don't want that. That said, I think a fix I put in to disable fwprop on these types may have been the culprit for that problem, so maybe we could add the CONST0_RTX back? I'd have to verify that. If so, then we'd at least be able to support -ftrivial-auto-var-init=zero. The =pattern version would be more problematical...unless the value for pattern was loaded from memory. Peter
> On Nov 30, 2021, at 9:14 AM, Peter Bergner <bergner@linux.ibm.com> wrote: > > On 11/30/21 2:37 AM, Richard Biener wrote: >> On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote: >> I think that's inconsistent indeed. Peter, what are "opaque" >> registers? rs6000-modes.def suggests >> that there's __vector_pair and __vector_quad, what's the GIMPLE types >> for those? It seems they >> are either SSA names or expanded to pseudo registers but there's no >> constants for them. > > The __vector_pair and __vector_quad types are target specific types > for use with our Matrix-Math-Assist (MMA) unit and they are only > usable with our associated MMA built-in functions. What they hold > is really dependent on which MMA built-ins you use on them. > You can think of them a generic (and large) vector type where the > subtype is undefined...or defined by which built-in function you > happen to be using. > > We do not have any constants defined for them. How we initialize them > is either by loading values from memory into them or by zeroing them > out using the xxsetaccz instruction (only for __vector_quads). So, looks like that the variable with OPAQUE_TYPE cannot be all explicitly initialized even at source code level. The only way to initialize such variable (only __vector_quad, not for __vector_pairs) at source code level is through call to __builtin_mma_xxsetaccz as: void foo (__vector_quad *dst) { __vector_quad acc; __builtin_mma_xxsetaccz(&acc); *dst = acc; } Is this the correct understanding? Is there way to initialize such variable to other values than zero at source code level? Qing > > > > >> Can they be initialized? I see they can be copied at least. > > __vector_quads can be zero initialized using the __builtin_mma_xxsetaccz() > built-in function. We don't have a method (or use case) for zero initializing > __vector_pairs. > > > >> If such "things" cannot be initialized they should indeed be exempt >> from auto-init. The >> documentation suggests that they act as bit-bucked but even bit-buckets should >> be initializable, thus why exactly does CONST0_RTX not exist for them? > > We used to have CONST0_RTX defined (but nothing else), but we had problems > with the compiler CSEing the initialization for multiple __vector_quads and > then copying the values around. We'd end up with one xxsetaccz instruction > and copies out of that accumulator register into the other accumulator > registers. Copies are VERY expensive, while xxsetaccz's are cheap, so we > don't want that. That said, I think a fix I put in to disable fwprop on > these types may have been the culprit for that problem, so maybe we could > add the CONST0_RTX back? I'd have to verify that. If so, then we'd at least > be able to support -ftrivial-auto-var-init=zero. The =pattern version > would be more problematical...unless the value for pattern was loaded from > memory. > > Peter > >
> On Nov 30, 2021, at 2:37 AM, Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Nov 29, 2021 at 11:56 PM Qing Zhao <qing.zhao@oracle.com> wrote: >> >> Peter, >> >> Thanks a lot for the patch. >> >> Richard, how do you think of the patch? >> >> (The major concern for me is: >> >> With the current patch proposed by Peter, we will generate the call to .DEFERRED_INIT for a variable with OPAQUE_TYPE during gimplification phase, >> However, if this variable is in register, then the call to .DEFERRED_INIT will NOT be expanded during RTL expansion phase. This unexpanded call to .DEFERRED_INIT might cause some potential IR issue later? > > I think that's inconsistent indeed. Can we treat the call to .DEFERRED_INIT to a NOP during expansion phase if we cannot expand it to a valid RTL for the OPAQUE_TYPE? Will doing this resolve the issues? > Peter, what are "opaque" > registers? rs6000-modes.def suggests > that there's __vector_pair and __vector_quad, what's the GIMPLE types > for those? It seems they > are either SSA names or expanded to pseudo registers but there's no > constants for them. > >> >> If the above is a real issue, should we skip initialization for all OPAQUE_TYPE variables even when they are in memory and can be initialized with memset? >> then we should update “is_var_need_auto_init” in gimplify.c instead. However, the issue with this approach is, we might miss the opportunity to initialize an OPAQUE_TYPE variable if it will be in memory? >> ). > > I think we need to bite the bullet at some point to do register initialization > not via expand_assignment but directly based on what the LHS expands to. OPAQUE_TYPE is so special, it should not be the reason to rewrite the register initialization from my understanding. If later more issue exposed, it might be necessary to rewrite this part. Qing > > Can they be initialized? I see they can be copied at least. > > If such "things" cannot be initialized they should indeed be exempt > from auto-init. The > documentation suggests that they act as bit-bucked but even bit-buckets should > be initializable, thus why exactly does CONST0_RTX not exist for them? > > Richard. > > >> >> Thanks. >> >> Qing >> >> >>> On Nov 29, 2021, at 3:56 PM, Peter Bergner <bergner@linux.ibm.com> wrote: >>> >>> Sorry for dropping the ball on testing the patch from the bugzilla! >>> >>> The following patch fixes the ICE reported in the bugzilla on the pre-existing >>> gcc testsuite test case, bootstraps and shows no testsuite regressions >>> on powerpc64le-linux. Ok for trunk? >>> >>> Peter >>> >>> >>> For -ftrivial-auto-var-init=*, skip initializing the register variable if it >>> is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes. >>> >>> gcc/ >>> PR middle-end/103127 >>> * internal-fn.c (expand_DEFERRED_INIT): Skip if VAR_TYPE is opaque. >>> >>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c >>> index 0cba95411a6..7cc0e9d5293 100644 >>> --- a/gcc/internal-fn.c >>> +++ b/gcc/internal-fn.c >>> @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt) >>> } >>> else >>> { >>> + /* Skip variables of opaque types that are in a register. */ >>> + if (OPAQUE_TYPE_P (var_type)) >>> + return; >>> + >>> /* If this variable is in a register use expand_assignment. >>> For boolean scalars force zero-init. */ >>> tree init; >>
On 11/30/21 11:51 AM, Qing Zhao wrote: > So, looks like that the variable with OPAQUE_TYPE cannot be all explicitly initialized even at source code level. > > The only way to initialize such variable (only __vector_quad, not for __vector_pairs) at source code level is through call to __builtin_mma_xxsetaccz as: > > void > foo (__vector_quad *dst) > { > __vector_quad acc; > __builtin_mma_xxsetaccz(&acc); > *dst = acc; > } > > Is this the correct understanding? Correct. Or via... > Is there way to initialize such variable to other values than zero at source code level? Not for any constant values. You can load it from memory though like below, which is also allowed for __vector_pair: void foo (__vector_quad *dst, __vector_quad *src) { __vector_quad acc; acc = *src; ... } void bar (__vector_pair *dst, __vector_pair *src) { __vector_pair pair; pair = *src; ... } We do not accept things like: acc = 0; acc = {0, 0, ... }; etc. Peter
> On Nov 30, 2021, at 12:08 PM, Peter Bergner <bergner@linux.ibm.com> wrote: > > On 11/30/21 11:51 AM, Qing Zhao wrote: >> So, looks like that the variable with OPAQUE_TYPE cannot be all explicitly initialized even at source code level. >> >> The only way to initialize such variable (only __vector_quad, not for __vector_pairs) at source code level is through call to __builtin_mma_xxsetaccz as: >> >> void >> foo (__vector_quad *dst) >> { >> __vector_quad acc; >> __builtin_mma_xxsetaccz(&acc); >> *dst = acc; >> } >> >> Is this the correct understanding? > > Correct. Or via... > > >> Is there way to initialize such variable to other values than zero at source code level? > > Not for any constant values. You can load it from memory though like below, > which is also allowed for __vector_pair: > > void > foo (__vector_quad *dst, __vector_quad *src) > { > __vector_quad acc; > acc = *src; > ... > } > void > bar (__vector_pair *dst, __vector_pair *src) > { > __vector_pair pair; > pair = *src; > ... > } However, even with the above, the memory pointed by “src” still need to be initialized somewhere. How to provide the initial value to the variable in the beginning for __vector_pair type? Qing > > We do not accept things like: > > acc = 0; > acc = {0, 0, ... }; > etc. > > Peter
On 11/30/21 1:50 PM, Qing Zhao via Gcc-patches wrote: >> void >> bar (__vector_pair *dst, __vector_pair *src) >> { >> __vector_pair pair; >> pair = *src; >> ... >> } > > However, even with the above, the memory pointed by “src” still need to > be initialized somewhere. How to provide the initial value to the variable > in the beginning for __vector_pair type? Well no initialization is required here in this function. Isn't that what matters here? When generating code for bar(), we assume that src already points to initialized memory. As for what src points to, that could be initialized how any other memory/ array could be initialized, so either a static array, read in some data from a file into an array, compute the array values in a loop, etc. etc. Peter
Sorry for the confusing… My major question is: for a variable of type __vector_pair, could it be in a register? If it could be in a register, can we initialize this register with some constant value? Qing > On Nov 30, 2021, at 2:07 PM, Peter Bergner <bergner@linux.ibm.com> wrote: > > On 11/30/21 1:50 PM, Qing Zhao via Gcc-patches wrote: >>> void >>> bar (__vector_pair *dst, __vector_pair *src) >>> { >>> __vector_pair pair; >>> pair = *src; >>> ... >>> } >> >> However, even with the above, the memory pointed by “src” still need to >> be initialized somewhere. How to provide the initial value to the variable >> in the beginning for __vector_pair type? > > Well no initialization is required here in this function. Isn't that what > matters here? When generating code for bar(), we assume that src already > points to initialized memory. > > As for what src points to, that could be initialized how any other memory/ > array could be initialized, so either a static array, read in some data > from a file into an array, compute the array values in a loop, etc. etc. > > Peter >
On 11/30/21 2:44 PM, Qing Zhao via Gcc-patches wrote: > Sorry for the confusing… > My major question is: > > for a variable of type __vector_pair, could it be in a register? Yes. To be pedantic, it will live in a vector register pair. > If it could be in a register, can we initialize this register with some constant value? For a __vector_pair, no, not as it is setup now. We also do not have a use case where we would want to initialize a __vector_pair to a constant. Our normal (only?) use case with a __vector_pair is to load it up with some actual data from memory that represents a (partial) row of a matrix. For __vector_quad, it too lives in a register (accumulator register) and represents a small matrix. We have the __builtin_mma_xxsetaccz (&acc) builtin to initialize it to a zero constant. Peter
On Tue, Nov 30, 2021 at 11:35 PM Peter Bergner <bergner@linux.ibm.com> wrote: > > On 11/30/21 2:44 PM, Qing Zhao via Gcc-patches wrote: > > Sorry for the confusing… > > My major question is: > > > > for a variable of type __vector_pair, could it be in a register? > > Yes. To be pedantic, it will live in a vector register pair. > > > > If it could be in a register, can we initialize this register with some constant value? > > For a __vector_pair, no, not as it is setup now. We also do not have a > use case where we would want to initialize a __vector_pair to a constant. > Our normal (only?) use case with a __vector_pair is to load it up with > some actual data from memory that represents a (partial) row of a matrix. > > For __vector_quad, it too lives in a register (accumulator register) and > represents a small matrix. We have the __builtin_mma_xxsetaccz (&acc) > builtin to initialize it to a zero constant. Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init instead of fixing up things at expansion time. Richard. > Peter >
> On Dec 1, 2021, at 3:01 AM, Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, Nov 30, 2021 at 11:35 PM Peter Bergner <bergner@linux.ibm.com> wrote: >> >> On 11/30/21 2:44 PM, Qing Zhao via Gcc-patches wrote: >>> Sorry for the confusing… >>> My major question is: >>> >>> for a variable of type __vector_pair, could it be in a register? >> >> Yes. To be pedantic, it will live in a vector register pair. >> >> >>> If it could be in a register, can we initialize this register with some constant value? >> >> For a __vector_pair, no, not as it is setup now. We also do not have a >> use case where we would want to initialize a __vector_pair to a constant. >> Our normal (only?) use case with a __vector_pair is to load it up with >> some actual data from memory that represents a (partial) row of a matrix. >> >> For __vector_quad, it too lives in a register (accumulator register) and >> represents a small matrix. We have the __builtin_mma_xxsetaccz (&acc) >> builtin to initialize it to a zero constant. > > Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init > instead of fixing up things at expansion time. Agreed. So, Peter, will you update the routine “is_var_need_auto_init” in gimplify.c to exclude OPAQUE_TYPE to fix this issue? thanks. Qing > > Richard. > >> Peter
On 12/1/21 9:06 AM, Qing Zhao wrote: >> On Dec 1, 2021, at 3:01 AM, Richard Biener <richard.guenther@gmail.com> wrote: >> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init >> instead of fixing up things at expansion time. > > Agreed. > > So, Peter, will you update the routine “is_var_need_auto_init” in gimplify.c > to exclude OPAQUE_TYPE to fix this issue? Sure, I can give that a try and verify it fixes the ICE too. Peter
On 12/1/21 3:01 AM, Richard Biener wrote: > Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init > instead of fixing up things at expansion time. The following fixes the ICE. The bootstrap/regtesting is still running though. Peter diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 8624f8221fd..326476f0238 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1829,6 +1829,7 @@ is_var_need_auto_init (tree decl) || !DECL_HARD_REGISTER (decl)) && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED) && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl))) + && !OPAQUE_TYPE_P (TREE_TYPE (decl)) && !is_empty_type (TREE_TYPE (decl))) return true; return false;
On December 1, 2021 6:42:21 PM GMT+01:00, Peter Bergner <bergner@linux.ibm.com> wrote: >On 12/1/21 3:01 AM, Richard Biener wrote: >> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init >> instead of fixing up things at expansion time. > >The following fixes the ICE. The bootstrap/regtesting is still running though. OK. Thanks, Richard. >Peter > > >diff --git a/gcc/gimplify.c b/gcc/gimplify.c >index 8624f8221fd..326476f0238 100644 >--- a/gcc/gimplify.c >+++ b/gcc/gimplify.c >@@ -1829,6 +1829,7 @@ is_var_need_auto_init (tree decl) > || !DECL_HARD_REGISTER (decl)) > && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED) > && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl))) >+ && !OPAQUE_TYPE_P (TREE_TYPE (decl)) > && !is_empty_type (TREE_TYPE (decl))) > return true; > return false;
On 12/1/21 1:07 PM, Richard Biener wrote: > On December 1, 2021 6:42:21 PM GMT+01:00, Peter Bergner <bergner@linux.ibm.com> wrote: >> On 12/1/21 3:01 AM, Richard Biener wrote: >>> Given all this I suggest to exempt OPAQUE_TYPE from is_var_need_auto_init >>> instead of fixing up things at expansion time. >> >> The following fixes the ICE. The bootstrap/regtesting is still running though. > > OK. Great, pushed. For posterity, below is what I committed. Peter middle-end: Skip initialization of opaque type variables [PR103127] For -ftrivial-auto-var-init=*, skip initializing the variable if it is an opaque type, because CONST0_RTX(mode) is not defined for opaque modes. 2021-12-01 Peter Bergner <bergner@linux.ibm.com> gcc/ PR middle-end/103127 * gimplify.c (is_var_need_auto_init): Handle opaque types. gcc/testsuite/ PR middle-end/103127 * gcc.target/powerpc/pr103127.c: New test. diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 8624f8221fd..326476f0238 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1829,6 +1829,7 @@ is_var_need_auto_init (tree decl) || !DECL_HARD_REGISTER (decl)) && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED) && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl))) + && !OPAQUE_TYPE_P (TREE_TYPE (decl)) && !is_empty_type (TREE_TYPE (decl))) return true; return false; diff --git a/gcc/testsuite/gcc.target/powerpc/pr103127.c b/gcc/testsuite/gcc.target/powerpc/pr103127.c new file mode 100644 index 00000000000..801fc0a4620 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr103127.c @@ -0,0 +1,19 @@ +/* PR target/103127 */ +/* { dg-require-effective-target power10_ok } */ +/* { dg-options "-O2 -mdejagnu-cpu=power10 -ftrivial-auto-var-init=zero" } */ + +/* Verify we do not ICE on the following tests. */ + +void +foo (__vector_quad *dst) +{ + __vector_quad acc; + *dst = acc; +} + +void +bar (__vector_pair *dst) +{ + __vector_pair pair; + *dst = pair; +}
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c index 0cba95411a6..7cc0e9d5293 100644 --- a/gcc/internal-fn.c +++ b/gcc/internal-fn.c @@ -3070,6 +3070,10 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt) } else { + /* Skip variables of opaque types that are in a register. */ + if (OPAQUE_TYPE_P (var_type)) + return; + /* If this variable is in a register use expand_assignment. For boolean scalars force zero-init. */ tree init;