Message ID | c7d29335-ac3a-24d4-486e-7755018263bf@linux.ibm.com |
---|---|
State | New, archived |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:ecc5:0:0:0:0:0 with SMTP id s5csp499903wro; Fri, 26 Aug 2022 20:50:50 -0700 (PDT) X-Google-Smtp-Source: AA6agR6H+kA1T2E3/XkqZcDxILzHPnK+LX5GrnO0WgDPtnIMxHj3ZY/h7KyWaiEkvP4/B/nSdiAP X-Received: by 2002:a17:906:8a49:b0:73d:cb3f:3401 with SMTP id gx9-20020a1709068a4900b0073dcb3f3401mr7159260ejc.184.1661572250557; Fri, 26 Aug 2022 20:50:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1661572250; cv=none; d=google.com; s=arc-20160816; b=RwtmL/oBGhvqc2Z6c2ukXbMfXMqZcCv/sWW1yYPIvZQuUDmhV3fMJB8tm12Cn84Mtt kjBbdP8HlPct4FFed+noYKLmu0s8SJ2CsLyM0Bcr5cuRk845nOGRKbhjhE8aCF2/fZ8X ZL4i2NLDpl8H0olGKfw3Y1s8bGWrfIMp+aPdvgP6FOhq14AxU7LQU3zcrIinKs4yKF1y eF16BHNiwUfvC8U8QdKS168ihVU1CKhV5v2ifVrXZnfTxcUKxq/JZLvk5hwfKisb6skI n02NIod07+1CABBgyTcnL0quek3uYzMzFhkiuKGG0dQwe0Zx83GVjwRkRzhlxLHenUh0 cVKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:subject:to:content-language:user-agent :mime-version:date:message-id:dmarc-filter:delivered-to :dkim-signature:dkim-filter; bh=DogwB+HTWAgEEPv/VnJcL6RNoFB4RqUz6ZTHdTU8UNA=; b=LCM8qoYIWaP1DvrqXNrqu+rv2Wz/PCkPSwREl2nIom7F+NVuBNR73sonDXuP6oOp8r sHH1lY2Fv0ABPx8W55aL9YNyJr9xZ87cKClFKqDWbf7B0CvWQlRsgxtDsliRbeQ343eW 3m2IBwe6YopNIqSGkXcVktPoQ/ckV88tb8MCsN9XltWXzQqeWGwF753KfDEPAIwjFtGz IA7XDG27ZQ8w075/kyAajMpKjUdstV0XFWI0woMQveMr+nXJ8wKx75afY5SKehau7h3i yLNFWVfsTXXunb0lmsDdTvieZqbSAqQ6bwAncUnXpCVWHkkhBRO+rARk+7F9SoB0HUpk pVTA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=ggrNnNiY; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id nc29-20020a1709071c1d00b0072b33152db1si2490370ejc.858.2022.08.26.20.50.50 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Aug 2022 20:50:50 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=ggrNnNiY; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5C09B382DB0F for <ouuuleilei@gmail.com>; Sat, 27 Aug 2022 03:50:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5C09B382DB0F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1661572249; bh=DogwB+HTWAgEEPv/VnJcL6RNoFB4RqUz6ZTHdTU8UNA=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=ggrNnNiYi+/DYAvHAxkAz8WkswJudmAyvn44mHsZNXfQccGOj0XxF3Wwqt5bL3omW D+pPohJ72EqCgtrp+UlyGYgZlQ8G0EKjrf3SZgzLLiAvl2CbeOroP7TvTPQodfowfP TWCGnQHO3slU2hF7+CpRECLpVHQzkdrjEbozerDs= 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 B2BDB382DB05; Sat, 27 Aug 2022 03:50:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B2BDB382DB05 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27R3koh2024275; Sat, 27 Aug 2022 03:50:04 GMT Received: from ppma01dal.us.ibm.com (83.d6.3fa9.ip4.static.sl-reverse.com [169.63.214.131]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3j7bnpr0u1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 27 Aug 2022 03:50:03 +0000 Received: from pps.filterd (ppma01dal.us.ibm.com [127.0.0.1]) by ppma01dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 27R3aqQP029911; Sat, 27 Aug 2022 03:50:02 GMT Received: from b01cxnp23033.gho.pok.ibm.com (b01cxnp23033.gho.pok.ibm.com [9.57.198.28]) by ppma01dal.us.ibm.com with ESMTP id 3j7aw908nb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 27 Aug 2022 03:50:02 +0000 Received: from b01ledav001.gho.pok.ibm.com (b01ledav001.gho.pok.ibm.com [9.57.199.106]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 27R3o18e5702266 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 27 Aug 2022 03:50:01 GMT Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 72C122805A; Sat, 27 Aug 2022 03:50:01 +0000 (GMT) Received: from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2766428059; Sat, 27 Aug 2022 03:50:01 +0000 (GMT) Received: from [9.160.4.32] (unknown [9.160.4.32]) by b01ledav001.gho.pok.ibm.com (Postfix) with ESMTP; Sat, 27 Aug 2022 03:50:01 +0000 (GMT) Message-ID: <c7d29335-ac3a-24d4-486e-7755018263bf@linux.ibm.com> Date: Fri, 26 Aug 2022 22:50:00 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Content-Language: en-US To: Segher Boessenkool <segher@gcc.gnu.org> Subject: [PATCH] rs6000: Don't ICE when we disassemble an MMA variable [PR101322] Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: ARy-xTLAYP0Pun8fZi9iosGtlABB8RSW X-Proofpoint-ORIG-GUID: ARy-xTLAYP0Pun8fZi9iosGtlABB8RSW X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-26_14,2022-08-25_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 clxscore=1011 mlxscore=0 adultscore=0 mlxlogscore=907 phishscore=0 bulkscore=0 impostorscore=0 lowpriorityscore=0 spamscore=0 malwarescore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2208270014 X-Spam-Status: No, score=-10.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, 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 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> Cc: GCC Patches <gcc-patches@gcc.gnu.org> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1742284784039522356?= X-GMAIL-MSGID: =?utf-8?q?1742284784039522356?= |
Series |
rs6000: Don't ICE when we disassemble an MMA variable [PR101322]
|
|
Commit Message
Peter Bergner
Aug. 27, 2022, 3:50 a.m. UTC
When we expand an MMA disassemble built-in with C++ using a pointer that is casted to a valid MMA type, the type isn't passed down to the expand machinery and we end up using the base type of the pointer which leads to an ICE. This patch enforces we always use the correct MMA type regardless of the pointer type being used. This passed bootstrap and regtesting on powerpc64le-linux with no regressions. Ok for trunk and backports after some burn-in time? Peter gcc/ PR target/101322 * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_mma_builtin): Enforce the use of a valid MMA pointer type. gcc/testsuite/ PR target/101322 * g++.target/powerpc/pr101322.C: New test.
Comments
Hi Peter, Thanks for the patch! Some comments are inline as below. on 2022/8/27 11:50, Peter Bergner via Gcc-patches wrote: > When we expand an MMA disassemble built-in with C++ using a pointer that > is casted to a valid MMA type, the type isn't passed down to the expand > machinery and we end up using the base type of the pointer which leads to > an ICE. This patch enforces we always use the correct MMA type regardless > of the pointer type being used. > > This passed bootstrap and regtesting on powerpc64le-linux with no regressions. > Ok for trunk and backports after some burn-in time? > > Peter > > gcc/ > PR target/101322 > * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_mma_builtin): > Enforce the use of a valid MMA pointer type. > > gcc/testsuite/ > PR target/101322 > * g++.target/powerpc/pr101322.C: New test. > > diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc > index 12afa86854c..e796e74f072 100644 > --- a/gcc/config/rs6000/rs6000-builtin.cc > +++ b/gcc/config/rs6000/rs6000-builtin.cc > @@ -1085,7 +1085,12 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi, > unsigned nvec = (fncode == RS6000_BIF_DISASSEMBLE_ACC) ? 4 : 2; > tree dst_ptr = gimple_call_arg (stmt, 0); > tree src_ptr = gimple_call_arg (stmt, 1); > - tree src_type = TREE_TYPE (src_ptr); > + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) > + ? build_pointer_type (vector_quad_type_node) > + : build_pointer_type (vector_pair_type_node); Nit: it seems we can use existing ptr_vector_quad_type_node and ptr_vector_pair_type_node? I assume the const qualifier is fine since it's for disassembling. > + if (TREE_TYPE (TREE_TYPE (src_ptr)) != src_type) This line looks unexpected, the former is type char while the latter is type __vector_pair *. I guess you meant to compare the type of pointer type like: TREE_TYPE (TREE_TYPE (src_ptr)) != TREE_TYPE (src_type) or even with mode like: TYPE_MODE (TREE_TYPE (TREE_TYPE (src_ptr))) != TYPE_MODE (TREE_TYPE (src_type)) > + src_ptr = build1 (VIEW_CONVERT_EXPR, src_type, src_ptr); Nit: NOP_EXPR seems to be better suited here for pointer conversion. BR, Kewen > + > tree src = create_tmp_reg_or_ssa_name (TREE_TYPE (src_type)); > gimplify_assign (src, build_simple_mem_ref (src_ptr), &new_seq); > > diff --git a/gcc/testsuite/g++.target/powerpc/pr101322.C b/gcc/testsuite/g++.target/powerpc/pr101322.C > new file mode 100644 > index 00000000000..59e71e8eb89 > --- /dev/null > +++ b/gcc/testsuite/g++.target/powerpc/pr101322.C > @@ -0,0 +1,17 @@ > +/* PR target/101322 */ > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */ > +/* { dg-require-effective-target power10_ok } */ > + > +/* Verify we don't ICE on the following test cases. */ > + > +void > +foo (char *resp, char *vpp) > +{ > + __builtin_vsx_disassemble_pair (resp, (__vector_pair *) vpp); > +} > + > +void > +bar (char *resp, char *vpp) > +{ > + __builtin_mma_disassemble_acc (resp, (__vector_quad *)vpp); > +}
On 8/31/22 4:22 AM, Kewen.Lin wrote: > on 2022/8/27 11:50, Peter Bergner via Gcc-patches wrote: >> - tree src_type = TREE_TYPE (src_ptr); >> + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) >> + ? build_pointer_type (vector_quad_type_node) >> + : build_pointer_type (vector_pair_type_node); > > Nit: it seems we can use existing ptr_vector_quad_type_node and ptr_vector_pair_type_node? > I assume the const qualifier is fine since it's for disassembling. That's actually what I started with, but I got some type of gimple verification error which I can't remember what it said. Let me put that back temporarily and I'll grab the error message. >> + if (TREE_TYPE (TREE_TYPE (src_ptr)) != src_type) > > This line looks unexpected, the former is type char while the latter is type __vector_pair *. > > I guess you meant to compare the type of pointer type like: > > TREE_TYPE (TREE_TYPE (src_ptr)) != TREE_TYPE (src_type) > > or even with mode like: > > TYPE_MODE (TREE_TYPE (TREE_TYPE (src_ptr))) != TYPE_MODE (TREE_TYPE (src_type)) Maybe? However, if that is the case, how can it be working for me? Let me throw this in the debugger and verify the types and I'll report back with what I find. >> + src_ptr = build1 (VIEW_CONVERT_EXPR, src_type, src_ptr); > > Nit: NOP_EXPR seems to be better suited here for pointer conversion. NOP_EXPR is new to me (I don't play in the middle-end that often). Let me look around at some other uses of it and I'll give it a try. Thanks! Peter
Hi! On Fri, Aug 26, 2022 at 10:50:00PM -0500, Peter Bergner wrote: > When we expand an MMA disassemble built-in with C++ using a pointer that > is casted to a valid MMA type, the type isn't passed down to the expand (The perfect tense of cast is "cast"). > machinery and we end up using the base type of the pointer which leads to > an ICE. This patch enforces we always use the correct MMA type regardless > of the pointer type being used. > --- /dev/null > +++ b/gcc/testsuite/g++.target/powerpc/pr101322.C > @@ -0,0 +1,17 @@ > +/* PR target/101322 */ > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */ > +/* { dg-require-effective-target power10_ok } */ These should be the other way around. It makes no sense to set option X before testing if you can set option X. Even if it would technically work, the primary consumer of any source code is the humans who read it, and humans read code top down. Segher
On 8/31/22 8:59 AM, Peter Bergner wrote: > On 8/31/22 4:22 AM, Kewen.Lin wrote: >> on 2022/8/27 11:50, Peter Bergner via Gcc-patches wrote: >>> - tree src_type = TREE_TYPE (src_ptr); >>> + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) >>> + ? build_pointer_type (vector_quad_type_node) >>> + : build_pointer_type (vector_pair_type_node); >> >> Nit: it seems we can use existing ptr_vector_quad_type_node and ptr_vector_pair_type_node? >> I assume the const qualifier is fine since it's for disassembling. > > That's actually what I started with, but I got some type of gimple > verification error which I can't remember what it said. Let me put > that back temporarily and I'll grab the error message. ...and of course, now I can't recreate that issue at all and the ptr_vector_*_type use work fine now. Strange! ...so ok, changed. Maybe the behavior changed since my PR106017 fix went in??? >>> + if (TREE_TYPE (TREE_TYPE (src_ptr)) != src_type) >> >> This line looks unexpected, the former is type char while the latter is type __vector_pair *. >> >> I guess you meant to compare the type of pointer type like: >> >> TREE_TYPE (TREE_TYPE (src_ptr)) != TREE_TYPE (src_type) > > Maybe? However, if that is the case, how can it be working for me? > Let me throw this in the debugger and verify the types and I'll report > back with what I find. Ok, you are correct. Thanks for catching that! I don't think we need those matching outer TREE_TYPE() uses. I think just a simple: if (TREE_TYPE (src_ptr) != src_type) ...should suffice. >> or even with mode like: >> >> TYPE_MODE (TREE_TYPE (TREE_TYPE (src_ptr))) != TYPE_MODE (TREE_TYPE (src_type)) I'd rather not look at the mode here, since OOmode/XOmode doesn't necessarily mean __vector_{pair,quad}, so I'll go with the modified test above. >>> + src_ptr = build1 (VIEW_CONVERT_EXPR, src_type, src_ptr); >> >> Nit: NOP_EXPR seems to be better suited here for pointer conversion. Ok, this works too, so code changed to use it. Thanks! Question for my own education, when would you use VIEW_CONVERT_EXPR over NOP_EXPR? FYI, here is the current code patch with all of the suggested changes incorporated: diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc index 12afa86854c..61352fcd801 100644 --- a/gcc/config/rs6000/rs6000-builtin.cc +++ b/gcc/config/rs6000/rs6000-builtin.cc @@ -1085,7 +1085,11 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi, unsigned nvec = (fncode == RS6000_BIF_DISASSEMBLE_ACC) ? 4 : 2; tree dst_ptr = gimple_call_arg (stmt, 0); tree src_ptr = gimple_call_arg (stmt, 1); - tree src_type = TREE_TYPE (src_ptr); + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) + ? ptr_vector_quad_type_node : ptr_vector_pair_type_node; + if (TREE_TYPE (src_ptr) != src_type) + src_ptr = build1 (NOP_EXPR, src_type, src_ptr); + tree src = create_tmp_reg_or_ssa_name (TREE_TYPE (src_type)); gimplify_assign (src, build_simple_mem_ref (src_ptr), &new_seq); I'll fire off a new round of bootstrap and regtesting and resubmit if it's clean. Thanks for the review! Peter
On Wed, Aug 31, 2022 at 01:53:48PM -0500, Peter Bergner wrote: > ...and of course, now I can't recreate that issue at all and the > ptr_vector_*_type use work fine now. Strange! ...so ok, changed. > Maybe the behavior changed since my PR106017 fix went in??? That is my best guess as well. But, how did that help this test? > Question for my own education, when would you use VIEW_CONVERT_EXPR over NOP_EXPR? VIEW_CONVERT_EXPR is essentially a bit_cast. Only use it when you need that, it is sub-optimal if you don't. > FYI, here is the current code patch with all of the suggested changes incorporated: > --- a/gcc/config/rs6000/rs6000-builtin.cc > +++ b/gcc/config/rs6000/rs6000-builtin.cc > @@ -1085,7 +1085,11 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi, > unsigned nvec = (fncode == RS6000_BIF_DISASSEMBLE_ACC) ? 4 : 2; > tree dst_ptr = gimple_call_arg (stmt, 0); > tree src_ptr = gimple_call_arg (stmt, 1); > - tree src_type = TREE_TYPE (src_ptr); > + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) > + ? ptr_vector_quad_type_node : ptr_vector_pair_type_node; If you split a?b:c over multiple lines, please make it three lines. Segher
On 8/31/22 3:51 PM, Segher Boessenkool wrote: > On Wed, Aug 31, 2022 at 01:53:48PM -0500, Peter Bergner wrote: >> Question for my own education, when would you use VIEW_CONVERT_EXPR over NOP_EXPR? > > VIEW_CONVERT_EXPR is essentially a bit_cast. Only use it when you need > that, it is sub-optimal if you don't. Ok. I believe I added a couple of other similar uses of VIEW_CONVERT_EXPR for pointer casts on the __builtin_vsx_lxvp/stxvp built-ins. I'll try converting those to use NOP_EXPR too in a separate cleanup patch. Thanks! >> - tree src_type = TREE_TYPE (src_ptr); >> + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) >> + ? ptr_vector_quad_type_node : ptr_vector_pair_type_node; > > If you split a?b:c over multiple lines, please make it three lines. Can do, however... >> ...and of course, now I can't recreate that issue at all and the >> ptr_vector_*_type use work fine now. Strange! ...so ok, changed. >> Maybe the behavior changed since my PR106017 fix went in??? > > That is my best guess as well. But, how did that help this test? It didn't. :-) During my bootstrap, I hit the gimple verification issue I mentioned seeing earlier. My problem was I thought I hit it with the test case, but it was exposed on a different test case in the testsuite. Here's what I'm seeing, which only happens when using -O0 -flto: rain6p1% gcc -O0 -mcpu=power10 -flto pr102347.c lto1: internal compiler error: in gimple_canonical_types_compatible_p, at tree.cc:13677 0x11930a97 gimple_canonical_types_compatible_p(tree_node const*, tree_node const*, bool) /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/tree.cc:13677 0x1192f1ab verify_type_variant /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/tree.cc:13377 0x11930beb verify_type(tree_node const*) /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/tree.cc:13700 0x106bbd37 lto_fixup_state /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto-common.cc:2629 0x106bbff3 lto_fixup_decls /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto-common.cc:2660 0x106bce13 read_cgraph_and_symbols(unsigned int, char const**) /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto-common.cc:2901 0x1067bcbf lto_main() /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto.cc:656 Please submit a full bug report, with preprocessed source (by using -freport-bug). Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. lto-wrapper: fatal error: /home/bergner/gcc/build/gcc-fsf-mainline-pr101322-debug/gcc/xgcc returned 1 exit status compilation terminated. /home/bergner/binutils/install/binutils-power10/bin/ld: error: lto-wrapper failed collect2: error: ld returned 1 exit status The problem goes away if I use use -O1 or above, I drop -flto or I use the code I originally posted without the ptr_vector_*_type The assert in gimple_canonical_types_compatible_p() we're hitting is: 13673 default: 13674 /* Consider all types with language specific trees in them mutually 13675 compatible. This is executed only from verify_type and false 13676 positives can be tolerated. */ 13677 gcc_assert (!in_lto_p); 13678 return true; I have no idea why ptr_vector_*_type would behave differently here than build_pointer_type (vector_*_type_node). Using the build_pointer_type() fixed it for me, so that's why I went with it. :-) Maybe this is a bug in lto??? Peter
On Wed, Aug 31, 2022 at 05:01:04PM -0500, Peter Bergner wrote: > The problem goes away if I use use -O1 or above, I drop -flto or I use > the code I originally posted without the ptr_vector_*_type > > The assert in gimple_canonical_types_compatible_p() we're hitting is: > 13673 default: > 13674 /* Consider all types with language specific trees in them mutually > 13675 compatible. This is executed only from verify_type and false > 13676 positives can be tolerated. */ > 13677 gcc_assert (!in_lto_p); > 13678 return true; > > I have no idea why ptr_vector_*_type would behave differently here than > build_pointer_type (vector_*_type_node). Using the build_pointer_type() > fixed it for me, so that's why I went with it. :-) Maybe this is a bug > in lto??? It looks like left-over debugging code. Or this truly should never happen in LTO? Segher
On 8/31/22 6:08 PM, Segher Boessenkool wrote: > On Wed, Aug 31, 2022 at 05:01:04PM -0500, Peter Bergner wrote: >> The problem goes away if I use use -O1 or above, I drop -flto or I use >> the code I originally posted without the ptr_vector_*_type >> >> The assert in gimple_canonical_types_compatible_p() we're hitting is: >> 13673 default: >> 13674 /* Consider all types with language specific trees in them mutually >> 13675 compatible. This is executed only from verify_type and false >> 13676 positives can be tolerated. */ >> 13677 gcc_assert (!in_lto_p); >> 13678 return true; >> >> I have no idea why ptr_vector_*_type would behave differently here than >> build_pointer_type (vector_*_type_node). Using the build_pointer_type() >> fixed it for me, so that's why I went with it. :-) Maybe this is a bug >> in lto??? > > It looks like left-over debugging code. Or this truly should never > happen in LTO? I have no idea. Either way, I'm going to go with the code that works, because either it's the "correct" code or it works around buggy code in lto. Either is a valid reason to me to use it. Peter
>>>> + if (TREE_TYPE (TREE_TYPE (src_ptr)) != src_type) >>> >>> This line looks unexpected, the former is type char while the latter is type __vector_pair *. >>> >>> I guess you meant to compare the type of pointer type like: >>> >>> TREE_TYPE (TREE_TYPE (src_ptr)) != TREE_TYPE (src_type) >> >> Maybe? However, if that is the case, how can it be working for me? >> Let me throw this in the debugger and verify the types and I'll report >> back with what I find. > > Ok, you are correct. Thanks for catching that! I don't think we need > those matching outer TREE_TYPE() uses. I think just a simple: > > if (TREE_TYPE (src_ptr) != src_type) > > ...should suffice. > Yeah, it's enough for the associated test case. :) > >>> or even with mode like: >>> >>> TYPE_MODE (TREE_TYPE (TREE_TYPE (src_ptr))) != TYPE_MODE (TREE_TYPE (src_type)) > > I'd rather not look at the mode here, since OOmode/XOmode doesn't necessarily > mean __vector_{pair,quad}, so I'll go with the modified test above. Good point. I thought the cv qualifier can affect the type equality check and assumed for test case like: void foo (char *resp, const __vector_pair *vpp) { __builtin_vsx_disassemble_pair (resp, (__vector_pair *) vpp); } , we don't want to have the conversion there and the ICE seems related to the underlying mode, so I thought maybe you wanted to use TYPE_MODE. >>>> + src_ptr = build1 (VIEW_CONVERT_EXPR, src_type, src_ptr); >>> >>> Nit: NOP_EXPR seems to be better suited here for pointer conversion. > > Ok, this works too, so code changed to use it. Thanks! > > Question for my own education, when would you use VIEW_CONVERT_EXPR over NOP_EXPR? tree.def has some note about VIEW_CONVERT_EXPR, it quite matches what Segher replied. In my experience, VIEW_CONVERT_EXPR are used a lot for vector type conversion. BR, Kewen
>>> ...and of course, now I can't recreate that issue at all and the >>> ptr_vector_*_type use work fine now. Strange! ...so ok, changed. >>> Maybe the behavior changed since my PR106017 fix went in??? >> >> That is my best guess as well. But, how did that help this test? > > It didn't. :-) During my bootstrap, I hit the gimple verification issue > I mentioned seeing earlier. My problem was I thought I hit it with the > test case, but it was exposed on a different test case in the testsuite. > Here's what I'm seeing, which only happens when using -O0 -flto: > > rain6p1% gcc -O0 -mcpu=power10 -flto pr102347.c > lto1: internal compiler error: in gimple_canonical_types_compatible_p, at tree.cc:13677 > 0x11930a97 gimple_canonical_types_compatible_p(tree_node const*, tree_node const*, bool) > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/tree.cc:13677 > 0x1192f1ab verify_type_variant > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/tree.cc:13377 > 0x11930beb verify_type(tree_node const*) > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/tree.cc:13700 > 0x106bbd37 lto_fixup_state > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto-common.cc:2629 > 0x106bbff3 lto_fixup_decls > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto-common.cc:2660 > 0x106bce13 read_cgraph_and_symbols(unsigned int, char const**) > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto-common.cc:2901 > 0x1067bcbf lto_main() > /home/bergner/gcc/gcc-fsf-mainline-pr101322/gcc/lto/lto.cc:656 > Please submit a full bug report, with preprocessed source (by using -freport-bug). > Please include the complete backtrace with any bug report. > See <https://gcc.gnu.org/bugs/> for instructions. > lto-wrapper: fatal error: /home/bergner/gcc/build/gcc-fsf-mainline-pr101322-debug/gcc/xgcc returned 1 exit status > compilation terminated. > /home/bergner/binutils/install/binutils-power10/bin/ld: error: lto-wrapper failed > collect2: error: ld returned 1 exit status > > The problem goes away if I use use -O1 or above, I drop -flto or I use > the code I originally posted without the ptr_vector_*_type > > The assert in gimple_canonical_types_compatible_p() we're hitting is: > 13673 default: > 13674 /* Consider all types with language specific trees in them mutually > 13675 compatible. This is executed only from verify_type and false > 13676 positives can be tolerated. */ > 13677 gcc_assert (!in_lto_p); > 13678 return true; > > I have no idea why ptr_vector_*_type would behave differently here than > build_pointer_type (vector_*_type_node). Using the build_pointer_type() > fixed it for me, so that's why I went with it. :-) Maybe this is a bug > in lto??? Thanks for your time to reproduce this! The only difference is that ptr_vector_*_type are built from the qualified_type based on vector_*_type_node, instead of directly from vector_*_type_node. I'm interested to have a further look at this later. BR, Kewen
On 9/1/22 3:29 AM, Kewen.Lin wrote: >> I have no idea why ptr_vector_*_type would behave differently here than >> build_pointer_type (vector_*_type_node). Using the build_pointer_type() >> fixed it for me, so that's why I went with it. :-) Maybe this is a bug >> in lto??? > > Thanks for your time to reproduce this! > > The only difference is that ptr_vector_*_type are built from the > qualified_type based on vector_*_type_node, instead of directly from > vector_*_type_node. I'm interested to have a further look at this later. If you look into this, please let me know. I'd like to know what you find out. Peter
On Thu, Sep 01, 2022 at 04:28:56PM +0800, Kewen.Lin wrote: > tree.def has some note about VIEW_CONVERT_EXPR, it quite matches what Segher replied. > In my experience, VIEW_CONVERT_EXPR are used a lot for vector type conversion. It is needed whenever vector types are not compatible otherwise. V4SI <-> V4SF, V4SI <-> V2DI, etc. In such cases you effectively do a bit_cast equivalent. Segher
on 2022/9/1 22:17, Peter Bergner wrote: > On 9/1/22 3:29 AM, Kewen.Lin wrote: >>> I have no idea why ptr_vector_*_type would behave differently here than >>> build_pointer_type (vector_*_type_node). Using the build_pointer_type() >>> fixed it for me, so that's why I went with it. :-) Maybe this is a bug >>> in lto??? >> >> Thanks for your time to reproduce this! >> >> The only difference is that ptr_vector_*_type are built from the >> qualified_type based on vector_*_type_node, instead of directly from >> vector_*_type_node. I'm interested to have a further look at this later. > > If you look into this, please let me know. I'd like to know what you > find out. I just filed PR106833 for it. BR, Kewen
diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc index 12afa86854c..e796e74f072 100644 --- a/gcc/config/rs6000/rs6000-builtin.cc +++ b/gcc/config/rs6000/rs6000-builtin.cc @@ -1085,7 +1085,12 @@ rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi, unsigned nvec = (fncode == RS6000_BIF_DISASSEMBLE_ACC) ? 4 : 2; tree dst_ptr = gimple_call_arg (stmt, 0); tree src_ptr = gimple_call_arg (stmt, 1); - tree src_type = TREE_TYPE (src_ptr); + tree src_type = (fncode == RS6000_BIF_DISASSEMBLE_ACC) + ? build_pointer_type (vector_quad_type_node) + : build_pointer_type (vector_pair_type_node); + if (TREE_TYPE (TREE_TYPE (src_ptr)) != src_type) + src_ptr = build1 (VIEW_CONVERT_EXPR, src_type, src_ptr); + tree src = create_tmp_reg_or_ssa_name (TREE_TYPE (src_type)); gimplify_assign (src, build_simple_mem_ref (src_ptr), &new_seq); diff --git a/gcc/testsuite/g++.target/powerpc/pr101322.C b/gcc/testsuite/g++.target/powerpc/pr101322.C new file mode 100644 index 00000000000..59e71e8eb89 --- /dev/null +++ b/gcc/testsuite/g++.target/powerpc/pr101322.C @@ -0,0 +1,17 @@ +/* PR target/101322 */ +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */ +/* { dg-require-effective-target power10_ok } */ + +/* Verify we don't ICE on the following test cases. */ + +void +foo (char *resp, char *vpp) +{ + __builtin_vsx_disassemble_pair (resp, (__vector_pair *) vpp); +} + +void +bar (char *resp, char *vpp) +{ + __builtin_mma_disassemble_acc (resp, (__vector_quad *)vpp); +}