From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id BE2793858D32 for ; Mon, 13 Mar 2023 06:55:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BE2793858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 32D5G7V6007333; Mon, 13 Mar 2023 06:55:41 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=EBHPJKzhxTV+Y3Y1JJk6jfd3zKsFO6/ZtPUc3ivksZI=; b=H43cNXLFsfYZpWKxImCZ1QVzkuhfB8g4WDdx5t+67auSLSvqHHMWhPvmDr8cG2uPEV4W RUN0MzNTxAqFUOCZkYFSr4SMltO4ia02ce7M2S1LyR1co4ftQnibsJMFmW83ZHZdILJ5 oZi9IZzFT+KBNbc2F1VAlvCNNs+w6s9mUJ3Z2KhibEpiKXze5wvKwjgPwTCuJZGMgmnq YZv1jbbiru1DOn3NectuElhM7sr4qzFbYCOUjGQNtCBRzIe5DoVcKR1hvCfnG+IsbqUs Liv87EUUl9JCUgccPOf+40paPzMcf4VOtWzVWWK8m5hOHI+zkCY5wq/iT4zVW0fKlLrx Lw== Received: from ppma04ams.nl.ibm.com (63.31.33a9.ip4.static.sl-reverse.com [169.51.49.99]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3p93t1chrp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 13 Mar 2023 06:55:40 +0000 Received: from pps.filterd (ppma04ams.nl.ibm.com [127.0.0.1]) by ppma04ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 32D2kiaU028609; Mon, 13 Mar 2023 06:55:39 GMT Received: from smtprelay07.fra02v.mail.ibm.com ([9.218.2.229]) by ppma04ams.nl.ibm.com (PPS) with ESMTPS id 3p8h96jwx3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 13 Mar 2023 06:55:39 +0000 Received: from smtpav01.fra02v.mail.ibm.com (smtpav01.fra02v.mail.ibm.com [10.20.54.100]) by smtprelay07.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 32D6tarm64422202 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 13 Mar 2023 06:55:36 GMT Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id ED6002006A; Mon, 13 Mar 2023 06:55:35 +0000 (GMT) Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5D41420043; Mon, 13 Mar 2023 06:55:34 +0000 (GMT) Received: from [9.200.40.78] (unknown [9.200.40.78]) by smtpav01.fra02v.mail.ibm.com (Postfix) with ESMTP; Mon, 13 Mar 2023 06:55:34 +0000 (GMT) Message-ID: Date: Mon, 13 Mar 2023 14:55:32 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] rs6000: Accept const pointer operands for MMA builtins [PR109073] Content-Language: en-US To: Segher Boessenkool Cc: Peter Bergner , Chip Kerchner , GCC Patches References: <40ecb0c8-2821-a72b-549d-6de6876b5d45@linux.ibm.com> <20230309145520.GU25951@gate.crashing.org> From: "Kewen.Lin" In-Reply-To: <20230309145520.GU25951@gate.crashing.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: ur6QHPumyazEZ1M3yqJAB-xw3zqBXTea X-Proofpoint-ORIG-GUID: ur6QHPumyazEZ1M3yqJAB-xw3zqBXTea X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-03-12_10,2023-03-10_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 bulkscore=0 clxscore=1015 mlxscore=0 phishscore=0 spamscore=0 suspectscore=0 mlxlogscore=878 impostorscore=0 adultscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2303130052 X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,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 List-Id: Hi Segher, on 2023/3/9 22:55, Segher Boessenkool wrote: > 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? For the test case (test.c) --- #include void foo (const __vector_pair *dst, __vector_pair *src, long idx) { __builtin_vsx_stxvp (*src, idx, dst); } void bar (const unsigned char *dst, vector unsigned char *src, long idx) { vec_xst (*src, idx, dst); } --- With *gcc-12 or trunk* (either cfe or c++fe), there is no warnings. With gcc-11: *cfe* test.c: In function ‘foo’: test.c:6:35: warning: passing argument 3 of ‘__builtin_vsx_stxvp’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 6 | __builtin_vsx_stxvp (*src, idx, dst); | ^~~ test.c:6:35: note: expected ‘__vector_pair *’ but argument is of type ‘const __vector_pair *’ test.c: In function ‘bar’: test.c:12:3: warning: passing argument 3 of ‘__builtin_vec_vsx_st’ discards qualifiers from pointer target type 12 | vec_xst (*src, idx, dst); | ^~~~~~~ *c++fe* test.c: In function ‘void foo(const __vector_pair*, __vector_pair*, long int)’: test.c:6:35: error: invalid conversion from ‘const __vector_pair*’ to ‘__vector_pair*’ [-fpermissive] 6 | __builtin_vsx_stxvp (*src, idx, dst); | ^~~ | | | const __vector_pair* : note: initializing argument 3 of ‘void __builtin_vsx_stxvp(__vector_pair, sizetype, __vector_pair*)’ test.c: In function ‘void bar(const unsigned char*, __vector unsigned char*, long int)’: test.c:12:11: warning: passing argument 3 of ‘__builtin_vec_vsx_st’ discards qualifiers from pointer target type 12 | vec_xst (*src, idx, dst); | ^ So for vec_xst, on gcc-11 which doesn't have new bif framework, there is always a warning. It looks that the new bif framework always makes those bif argument pointer types with const qualifier. A quick scanning on function rs6000_init_builtins (trunk code) didn't find a counter case. The difference on pointers of types __vector_pair* and vector unsigned char* made me wonder if the const qualifier on __builtin_vsx_stxvp is intentional, maybe opaque types are different? since we get warning for vec_xst but get error for __builtin_vsx_stxvp even without new bif framework. BR, Kewen