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 07ADC38582AF for ; Mon, 17 Oct 2022 12:55:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 07ADC38582AF 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 (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29HCsOHF032648; Mon, 17 Oct 2022 12:55:34 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=cF6WUKvjTKPicRA5ISZULOupq19hbvekKAj1wz9aMe4=; b=T3v0N8z/Eyd6qVFSesza2aEDOqPPgjty36kEBs8eiyS4LHzGPrRD/DQwEE8SECjaVJeC wWCvMsrdrYiIg/9zxxh2HW+wWePmKFt8DDE09jZR66FR7H1yMbg765reOGKwpqs/Y46q JTWgQ3svziSBIoCV94VwNabjlUJQbYT8O0hY1Fu/4KfwA1eWL0OiNq4H254thOnuK8Z9 bkuc8WearQMf2SSzYmenvMVDvLvTeKScbmKDKgrvAATY3NRvYrkoECwDE2d9w+8WL5O6 Uw3WRtY//CcotDUnMNwnvDx9asrV+y8/Yz44EfotSJ+CUcas0EFy1rCcXhTmi1U7mXge BQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3k86x8gu46-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 17 Oct 2022 12:55:34 +0000 Received: from m0098420.ppops.net (m0098420.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 29HCdbgn001237; Mon, 17 Oct 2022 12:55:34 GMT Received: from ppma02fra.de.ibm.com (47.49.7a9f.ip4.static.sl-reverse.com [159.122.73.71]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3k86x8gu3c-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 17 Oct 2022 12:55:33 +0000 Received: from pps.filterd (ppma02fra.de.ibm.com [127.0.0.1]) by ppma02fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 29HCaE9O028084; Mon, 17 Oct 2022 12:55:32 GMT Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by ppma02fra.de.ibm.com with ESMTP id 3k7mg929fg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 17 Oct 2022 12:55:31 +0000 Received: from d06av22.portsmouth.uk.ibm.com (d06av22.portsmouth.uk.ibm.com [9.149.105.58]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 29HCtTuV61276496 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 17 Oct 2022 12:55:29 GMT Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 876304C04A; Mon, 17 Oct 2022 12:55:29 +0000 (GMT) Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 291514C044; Mon, 17 Oct 2022 12:55:27 +0000 (GMT) Received: from [9.200.39.63] (unknown [9.200.39.63]) by d06av22.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 17 Oct 2022 12:55:26 +0000 (GMT) Message-ID: <9b3c6d56-497a-5460-a82d-37917f323900@linux.ibm.com> Date: Mon, 17 Oct 2022 20:55:25 +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] Split TARGET_POWER8 from TARGET_DIRECT_MOVE [PR101865] (2/2) Content-Language: en-US To: will schmidt Cc: Segher Boessenkool , David Edelsohn , Michael Meissner , GCC patches References: <2656f0182df289ade33411ac579d2d5a229f5e4c.camel@vnet.ibm.com> From: "Kewen.Lin" In-Reply-To: <2656f0182df289ade33411ac579d2d5a229f5e4c.camel@vnet.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: dJmP1kop5gNosR2V3aZ3Se7m0FIw89ZU X-Proofpoint-GUID: JUtHf3c65f2M0cG8JCG2WZhbrmjrMBLp 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-17_09,2022-10-17_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 mlxlogscore=999 spamscore=0 adultscore=0 priorityscore=1501 suspectscore=0 lowpriorityscore=0 clxscore=1015 malwarescore=0 bulkscore=0 impostorscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2210170072 X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,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 Will, Thanks for fixing this, some comments are inline as below. on 2022/9/20 00:13, will schmidt wrote: > [PATCH, rs6000] Split TARGET_POWER8 from TARGET_DIRECT_MOVE [PR101865] > > Hi, > 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. > > This is done via the OPTION_MASK definitions, so this > means that some references to the OPTION_MASK_DIRECT_MOVE > option are now replaced with OPTION_MASK_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. The mentioned commentary for DIRECT_MOVE looks out of date since option direct_move is marked as Undocumented & WarnRemoved, it can't be enabled/disabled explicitly. Personally I'm inclined not to introduce __DIRECT_MOVE__ define, since we don't have a separated option for it now, and if users want to check the availability, they can check __VSX__ && _ARCH_PWR8 instead. > > This regests cleanly (power8,power9,power10), and resolves > PR 101865 as represented in the tests from (1/2). > > OK for trunk? > Thanks, > -Will > > > gcc/ > PR Target/101865 > * config/rs6000/rs6000-builtin.cc > (rs6000_builtin_is_supported): Replace TARGET_DIRECT_MOVE > usage with TARGET_POWER8. > * config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): > Add __DIRECT_MOVE__ define. Replace _ARCH_PWR8_ define > conditional with OPTION_MASK_POWER8. > * config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER): > Add OPTION_MASK_POWER8 entry. > (POWERPC_MASKS): Same. > * config/rs6000/rs6000.cc (rs6000_option_override_internal): > Replace OPTION_MASK_DIRECT_MOVE usage with OPTION_MASK_POWER8. > (rs6000_opt_masks): Add "power8" entry for new OPTION_MASK_POWER8. > * config/rs6000/rs6000.opt (-mpower8): Add entry for POWER8. > * config/rs6000/vsx.md (vsx_extract_): Replace > TARGET_DIRECT_MOVE usage with TARGET_POWER8. > (define_peephole2): Same. > > diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc > index 3ce729c1e6de..91a0f39bd796 100644 > --- a/gcc/config/rs6000/rs6000-builtin.cc > +++ b/gcc/config/rs6000/rs6000-builtin.cc > @@ -163,11 +163,11 @@ rs6000_builtin_is_supported (enum rs6000_gen_builtins fncode) > case ENB_P7: > return TARGET_POPCNTD; > case ENB_P7_64: > return TARGET_POPCNTD && TARGET_POWERPC64; > case ENB_P8: > - return TARGET_DIRECT_MOVE; > + return TARGET_POWER8; > case ENB_P8V: > return TARGET_P8_VECTOR; > case ENB_P9: > return TARGET_MODULO; > case ENB_P9_64: > diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc > index ca9cc42028f7..41d51b039061 100644 > --- a/gcc/config/rs6000/rs6000-c.cc > +++ b/gcc/config/rs6000/rs6000-c.cc > @@ -439,11 +439,13 @@ rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags) > turned off in any of the following conditions: > 1. TARGET_HARD_FLOAT, TARGET_ALTIVEC, or TARGET_VSX is explicitly > disabled and OPTION_MASK_DIRECT_MOVE was not explicitly > enabled. > 2. TARGET_VSX is off. */ As mentioned above, the comments might need some updates. > - if ((flags & OPTION_MASK_DIRECT_MOVE) != 0) > + if ((OPTION_MASK_DIRECT_MOVE) != 0) > + rs6000_define_or_undefine_macro (define_p, "__DIRECT_MOVE__"); > + if ((flags & OPTION_MASK_POWER8) != 0) > rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR8"); > if ((flags & OPTION_MASK_MODULO) != 0) > rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR9"); > if ((flags & OPTION_MASK_POWER10) != 0) > rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR10"); > diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def > index c3825bcccd84..c873f6d58989 100644 > --- a/gcc/config/rs6000/rs6000-cpus.def > +++ b/gcc/config/rs6000/rs6000-cpus.def > @@ -48,10 +48,11 @@ > system. */ > #define ISA_2_7_MASKS_SERVER (ISA_2_6_MASKS_SERVER \ > | OPTION_MASK_P8_VECTOR \ > | OPTION_MASK_CRYPTO \ > | OPTION_MASK_DIRECT_MOVE \ > + | OPTION_MASK_POWER8 \ > | OPTION_MASK_EFFICIENT_UNALIGNED_VSX \ > | OPTION_MASK_QUAD_MEMORY \ > | OPTION_MASK_QUAD_MEMORY_ATOMIC) > > /* ISA masks setting fusion options. */ > @@ -124,10 +125,11 @@ > #define POWERPC_MASKS (OPTION_MASK_ALTIVEC \ > | OPTION_MASK_CMPB \ > | OPTION_MASK_CRYPTO \ > | OPTION_MASK_DFP \ > | OPTION_MASK_DIRECT_MOVE \ > + | OPTION_MASK_POWER8 \ > | OPTION_MASK_DLMZB \ > | OPTION_MASK_EFFICIENT_UNALIGNED_VSX \ > | OPTION_MASK_FLOAT128_HW \ > | OPTION_MASK_FLOAT128_KEYWORD \ > | OPTION_MASK_FPRND \ > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index fcca062a8709..ed423b9e1837 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -3766,15 +3766,14 @@ rs6000_option_override_internal (bool global_init_p) > if ((rs6000_isa_flags_explicit & OPTION_MASK_MULTIPLE) != 0) > warning (0, "%qs is not supported on little endian systems", > "-mmultiple"); > } > > - /* If little-endian, default to -mstrict-align on older processors. > - Testing for direct_move matches power8 and later. */ > + /* If little-endian, default to -mstrict-align on older processors. */ > if (!BYTES_BIG_ENDIAN > && !(processor_target_table[tune_index].target_enable > - & OPTION_MASK_DIRECT_MOVE)) > + & OPTION_MASK_POWER8)) > rs6000_isa_flags |= ~rs6000_isa_flags_explicit & OPTION_MASK_STRICT_ALIGN; > > /* Add some warnings for VSX. */ > if (TARGET_VSX) > { > @@ -3857,11 +3856,11 @@ rs6000_option_override_internal (bool global_init_p) > error ("%qs incompatible with explicitly disabled options", > "-mpower9-minmax"); > else > rs6000_isa_flags |= ISA_3_0_MASKS_SERVER; > } > - else if (TARGET_P8_VECTOR || TARGET_DIRECT_MOVE || TARGET_CRYPTO) > + else if (TARGET_P8_VECTOR || TARGET_POWER8 || TARGET_CRYPTO) > rs6000_isa_flags |= (ISA_2_7_MASKS_SERVER & ~ignore_masks); > else if (TARGET_VSX) > rs6000_isa_flags |= (ISA_2_6_MASKS_SERVER & ~ignore_masks); > else if (TARGET_POPCNTD) > rs6000_isa_flags |= (ISA_2_6_MASKS_EMBEDDED & ~ignore_masks); > @@ -24046,10 +24045,11 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] = > { "block-ops-vector-pair", OPTION_MASK_BLOCK_OPS_VECTOR_PAIR, > false, true }, > { "cmpb", OPTION_MASK_CMPB, false, true }, > { "crypto", OPTION_MASK_CRYPTO, false, true }, > { "direct-move", OPTION_MASK_DIRECT_MOVE, false, true }, > + { "power8", OPTION_MASK_POWER8, false, true }, > { "dlmzb", OPTION_MASK_DLMZB, false, true }, > { "efficient-unaligned-vsx", OPTION_MASK_EFFICIENT_UNALIGNED_VSX, > false, true }, > { "float128", OPTION_MASK_FLOAT128_KEYWORD, false, true }, > { "float128-hardware", OPTION_MASK_FLOAT128_HW, false, true }, > diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt > index b63a5d443af6..53964387da6d 100644 > --- 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). Since this option is marked as "Undocumented", I think it doesn't need the documentation here. And it seems not a good idea to make this option back settable by users. > + > +mpower8 > +Target Mask(POWER8) Var(rs6000_isa_flags) > +Use instructions added in ISA 2.07 (power8). > We don't allow users to specify -mpower10, so I think we don't want to allow it for power8 too. So can we mark it as "Undocumented" and "WarnRemoved" (or similar to ensure users can't specify it)? > mhtm > Target Mask(HTM) Var(rs6000_isa_flags) > Use ISA 2.07 transactional memory (HTM) instructions. > > diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md > index e226a93bbe55..be4fb902049d 100644 > --- 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) > return "mfvsrd %0,%x1"; The context here indicates the TARGET_DIRECT_MOVE use is expected here, as it's moving from VSX register to GPR directly, no? > > else if (FP_REGNO_P (op0_regno) && FP_REGNO_P (op1_regno)) > return "fmr %0,%1"; > @@ -6204,11 +6204,11 @@ (define_peephole2 > > ;; MTVSRD > (set (match_operand:SF SFBOOL_MTVSR_D "vsx_register_operand") > (unspec:SF [(match_dup SFBOOL_SHL_D)] UNSPEC_P8V_MTVSRD))] > > - "TARGET_POWERPC64 && TARGET_DIRECT_MOVE > + "TARGET_POWERPC64 && TARGET_POWER8 Similar here, could you have a double check? > /* The REG_P (xxx) tests prevents SUBREG's, which allows us to use REGNO > to compare registers, when the mode is different. */ > && REG_P (operands[SFBOOL_MFVSR_D]) && REG_P (operands[SFBOOL_BOOL_D]) > && REG_P (operands[SFBOOL_BOOL_A1]) && REG_P (operands[SFBOOL_SHL_D]) > && REG_P (operands[SFBOOL_SHL_A]) && REG_P (operands[SFBOOL_MTVSR_D]) > Besides, one place in rs6000.md that caught my eyes and requires an update seems to be: (define_insn "prefetch" [(prefetch (match_operand 0 "indexed_or_indirect_address" "a") (match_operand:SI 1 "const_int_operand" "n") (match_operand:SI 2 "const_int_operand" "n"))] "" { /* dcbtstt, dcbtt and TH=0b10000 support starts with ISA 2.06 (Power7). AIX does not support the dcbtstt and dcbtt extended mnemonics. The AIX assembler does not support the three operand form of dcbt and dcbtst on Power 7 (-mpwr7). */ int inst_select = INTVAL (operands[2]) || !TARGET_DIRECT_MOVE; As the changelog of the related commit, TARGET_DIRECT_MOVE here is used to guard power8. BR, Kewen