From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 1B4B03857839 for ; Tue, 18 Oct 2022 15:17:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1B4B03857839 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vnet.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vnet.ibm.com Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29IEDwUq017210; Tue, 18 Oct 2022 15:17:34 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : to : cc : date : in-reply-to : references : content-type : mime-version : content-transfer-encoding; s=pp1; bh=zg46NJr/GKLks1U/fx53MII872u5KDiryG6n25zfQOE=; b=K9a1FDa7ZZoG8ST7CeJdUW3f9/oFWKPxY2eOAAOhhB+FPUCqgVCvACEC+squmGI1dCIw hLa/ugJsJKYOPu2yuM2C/H1gaF1QnG6TpC4B4hUqEl/Gap6rEDEZgJTiVd1HwL48jHMW XwJpMXhSnKFz/QnNTvtMpKWJFxFhdGmP/79uPXk2A889qJp92bgFrO4wQMn0aGHRbrS7 ZR4GlHbe9p6hZlpIYP2vdbeB7B5o+RR1QQ75izY6OmU8doY1p+1Umau4mm6CX/v++JBz 3wu4w31WSWlzvuQMBce8bPvwlyYg43CCfPzJSKGArC4DGCXMhfgf3qMyVcUzWYELiHhg TQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3k9r2ag2pq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 18 Oct 2022 15:17:34 +0000 Received: from m0098409.ppops.net (m0098409.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 29IEGsQj000698; Tue, 18 Oct 2022 15:17:33 GMT Received: from ppma01wdc.us.ibm.com (fd.55.37a9.ip4.static.sl-reverse.com [169.55.85.253]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3k9r2ag2nf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 18 Oct 2022 15:17:33 +0000 Received: from pps.filterd (ppma01wdc.us.ibm.com [127.0.0.1]) by ppma01wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 29IFF9cE029487; Tue, 18 Oct 2022 15:17:32 GMT Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by ppma01wdc.us.ibm.com with ESMTP id 3k9be2a1f7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 18 Oct 2022 15:17:32 +0000 Received: from smtpav04.dal12v.mail.ibm.com ([9.208.128.131]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 29IFHUbP34341362 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 18 Oct 2022 15:17:30 GMT Received: from smtpav04.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 45B225805A; Tue, 18 Oct 2022 15:17:31 +0000 (GMT) Received: from smtpav04.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9B77758056; Tue, 18 Oct 2022 15:17:30 +0000 (GMT) Received: from sig-9-65-213-31.ibm.com (unknown [9.65.213.31]) by smtpav04.dal12v.mail.ibm.com (Postfix) with ESMTP; Tue, 18 Oct 2022 15:17:30 +0000 (GMT) Message-ID: Subject: Re: [PATCH, rs6000] Split TARGET_POWER8 from TARGET_DIRECT_MOVE [PR101865] (2/2) From: will schmidt To: Segher Boessenkool Cc: GCC patches , David Edelsohn , "Kewen.Lin" , Michael Meissner Date: Tue, 18 Oct 2022 10:17:30 -0500 In-Reply-To: <20221017180840.GJ25951@gate.crashing.org> References: <2656f0182df289ade33411ac579d2d5a229f5e4c.camel@vnet.ibm.com> <20221017180840.GJ25951@gate.crashing.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: yzbvaUjG6oE5hnOJoP4c1rEjerw4tEPR X-Proofpoint-GUID: 6RgTRcNKwsm7fPv4dPof8X8elSRLvfif X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-10-18_05,2022-10-18_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 impostorscore=0 malwarescore=0 spamscore=0 mlxscore=0 lowpriorityscore=0 priorityscore=1501 adultscore=0 mlxlogscore=999 clxscore=1015 suspectscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2210180083 X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,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: On Mon, 2022-10-17 at 13:08 -0500, Segher Boessenkool wrote: > On Mon, Sep 19, 2022 at 11:13:20AM -0500, will schmidt wrote: > > The _ARCH_PWR8 define is conditional on TARGET_DIRECT_MOVE, > > and can be disabled by dependent options when it should not be. > > This manifests in the issue seen in PR101865 where -mno-vsx > > mistakenly disables _ARCH_PWR8. > > This change replaces the relevant TARGET_DIRECT_MOVE references > > with a TARGET_POWER8 entry so that the direct_move and power8 > > features can be enabled or disabled independently. > > We should get rid of TARGET_DIRECT_MOVE altogether. Please see > 57f108f5a1e1: > rs6000: Disable -m[no-]direct-move (PR85293) > > The -mno-direct-move option causes a lot of problems, since it > forces > us to be able to generate code for p8 and up with some crucial > instructions missing. This patch removes the -m[no-]direct-move > options so that the user cannot put us into this unexpected > situation > anymore. Internally we still have all the same flags, and they > are > automatically set based on -mcpu; getting rid of that is a lot > more > work and will have to wait for GCC 9 (in some places the flag is > used > to see if we are compiling for a p8 _at all_). > > It did not happen in GCC 9 obviously. Do you want to take a > shot? It > doesn't have to be all at once, it's probably best if not even -- as > I > wrote in the commit message, the flag always was used to mean > different > things. As long as it's OK to be removed, I'll certainly take a shot at it. With that in mind that may simplify things for me here. I expect that anything currently guarded by DIRECT_MOVE should instead be guarded by POWER8. > > > The existing (and rather lengthy) commentary for DIRECT_MOVE > > remains > > in place in rs6000-c.cc:rs6000_target_modify_macros(). The > > if-defined logic there will now set a __DIRECT_MOVE__ define when > > TARGET_DIRECT_MOVE is set, this serves as a placeholder for debug > > purposes, but is otherwise unused. This can be removed in a > > subsequent patch, or in an update of this patch, depending on > > feedback. > > There should be no such macro, for the same reason there should be no > -mdirect-move option: it is so very essential to all code we > generate, > it *always* is enabled if we have P8 or later. fair enough. > > > gcc/ > > PR Target/101865 > > * config/rs6000/rs6000-builtin.cc > > (rs6000_builtin_is_supported): Replace TARGET_DIRECT_MOVE > > usage with TARGET_POWER8. > > Please don't arbitrarily wrap lines. It is harder to read, and it > looks > like something is missing. > > > * config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER): > > Add OPTION_MASK_POWER8 entry. > > Especially in cases like this, where it looks like you forgot to > write > something after the colon. > > > @@ -24046,10 +24045,11 @@ static struct rs6000_opt_mask const > > rs6000_opt_masks[] = > > { "block-ops-vector-pair", OPTION_MASK_BLOCK_OPS_VECTOR_PA > > IR, > > false, > > true }, > > { "cmpb", OPTION_MASK_CMPB, fal > > se, true }, > > { "crypto", OPTION_MASK_CRYPTO, fal > > se, true }, > > { "direct-move", OPTION_MASK_DIRECT_MOVE, false, > > true }, > > + { "power8", OPTION_MASK_POWER8, fal > > se, true }, > > Why would we want a #pragma power8 ? Hmm, thinko on my part, i'll reevaluate. > > > --- a/gcc/config/rs6000/rs6000.opt > > +++ b/gcc/config/rs6000/rs6000.opt > > @@ -490,10 +490,15 @@ mcrypto > > Target Mask(CRYPTO) Var(rs6000_isa_flags) > > Use ISA 2.07 Category:Vector.AES and Category:Vector.SHA2 > > instructions. > > > > mdirect-move > > Target Undocumented Mask(DIRECT_MOVE) Var(rs6000_isa_flags) > > WarnRemoved > > +Enable direct move (ISA 2.07). > > It is undocumented and should remain that, except eventually we > should > remove it completely (but leave some stubs so that code in the wild > keeps compiling). > > > +mpower8 > > +Target Mask(POWER8) Var(rs6000_isa_flags) > > +Use instructions added in ISA 2.07 (power8). > > There should not be such an option. It is set by -mcpu=power8 and > later, but can never be enabled or disabled direfctly by the user. OK. Thanks for the detailed review. :-) -Will > > > --- a/gcc/config/rs6000/vsx.md > > +++ b/gcc/config/rs6000/vsx.md > > @@ -3407,11 +3407,11 @@ (define_insn "vsx_extract_" > > if (element == VECTOR_ELEMENT_SCALAR_64BIT) > > { > > if (op0_regno == op1_regno) > > return ASM_COMMENT_START " vec_extract to same register"; > > > > - else if (INT_REGNO_P (op0_regno) && TARGET_DIRECT_MOVE > > + else if (INT_REGNO_P (op0_regno) && TARGET_POWER8 > > && TARGET_POWERPC64) > > That fits on one line now. > > Thanks, > > > Segher