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 8C42B3858D33 for ; Thu, 9 Mar 2023 14:56:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8C42B3858D33 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 329EtSJC019299; Thu, 9 Mar 2023 08:55:28 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 329EtLms019296; Thu, 9 Mar 2023 08:55:21 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Thu, 9 Mar 2023 08:55:20 -0600 From: Segher Boessenkool To: "Kewen.Lin" Cc: Peter Bergner , Chip Kerchner , GCC Patches Subject: Re: [PATCH] rs6000: Accept const pointer operands for MMA builtins [PR109073] Message-ID: <20230309145520.GU25951@gate.crashing.org> References: <40ecb0c8-2821-a72b-549d-6de6876b5d45@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,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 List-Id: Hi! On Thu, Mar 09, 2023 at 05:30:53PM +0800, Kewen.Lin wrote: > on 2023/3/9 07:01, Peter Bergner via Gcc-patches wrote: > > PR109073 shows a problem where GCC 11 and GCC 10 do not accept a const > > __vector_pair pointer operand to some MMA builtins, which GCC 12 and later > > correctly accept. Fixed here by initializing the builtins to accept const > > pointers. "Pointers to const" is the more correct. A "const pointer" is e.g. int *const p; not the same thing at all, and sometimes this is useful to have ;-) > > This patch was tested in both GCC 11 and GCC 10 on powerpc64le-linux and > > showed no regressions. Ok for backports? It isn't truly a backport. You can put it on 11 and 10 at the same time, there is no benefit doing it on 11 only first. > > { > > op[nopnds++] = build_pointer_type (void_type_node); > > if (d->code == MMA_BUILTIN_DISASSEMBLE_ACC) > > - op[nopnds++] = build_pointer_type (vector_quad_type_node); > > + op[nopnds++] = build_pointer_type (build_qualified_type > > + (vector_quad_type_node, > > + TYPE_QUAL_CONST)); > > Nit: Maybe we can build them out of the loop once and then just use the > built one in the loop. Or as globals even. Currently we have X and pointer to X, but no pointer to const X (and no const X either, but that isn't so useful). The generic code doesn't have this either, hrm. (snip) > Simply testing __builtin_mma_xxmtacc and __builtin_mma_xxmfacc as below: > > $ cat test.C > void foo0(const __vector_quad *acc) { > __builtin_mma_xxmtacc(acc); > __builtin_mma_xxmfacc(acc); > } > > test.C:2:25: error: invalid conversion from ‘const __vector_quad*’ to ‘__vector_quad*’ [-fpermissive] > 2 | __builtin_mma_xxmtacc(acc); > > test.C:3:25: error: invalid conversion from ‘const __vector_quad*’ to ‘__vector_quad*’ [-fpermissive] > 3 | __builtin_mma_xxmfacc(acc); > > They also suffered the same error on gcc11 branch but not on trunk. Yeah, there is more to be done here. > Besides, I'm not sure if the existing bif declarations using ptr_vector_pair_type_node > and ptr_vector_quad_type_node are all intentional, at least it looks weird to me that > we declare const __vector_pair* for this __builtin_vsx_stxvp, which is meant to store 32 > bytes into the memory provided by the pointer biasing the sizetype offset, but the "const" > qualifier seems to tell that this bif doesn't modify the memory pointed by the given pointer. That looks like a bug. Well it is one even. Is it fixed on trunk? Since the patch is a strict improvement already, it is okay for 11 and 10. But you (Peter) may want to flesh it out a bit first? Or first commit only this if that works better for you. Segher