From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 4D7973858C53 for ; Wed, 7 Feb 2024 09:38:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4D7973858C53 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 ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4D7973858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707298738; cv=none; b=IEC0H0kPbn4wNeDFj/88lUhD1tHpeVsxwkWZahv1ycrDJ3YBsm2UqPKMltnVREQ/4h5uyD+kdsXv/tKye0i6X6BEUkjavr2LjRP7wwPUHXOSb54P3dklfzeFBLJbe8M8MQyo7+aJZ+lUhorVbMlMBulSdqAdYJSI/KDxIe34j/A= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707298738; c=relaxed/simple; bh=G2jG4hfeIiUren/f9PvXemwyNK4AYVMA476zp58kqIk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:From:Subject:To; b=oMi3TZxNQ5Ibo7UJpym7kuDs3BGRMJAXV0dD8Ozvs62FyBXsgzat75VSDCE8k3SHNF6sG8nYMigN7UbjaioAHG1N4tNXpx6bXSkYKYqkj3FdPd18/J/+tYYLdYAnzqeWLqln9Zs/WolZpoqjIp7OR64N5tXkG/b/QI4/DK+49ls= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 417938ZF006045; Wed, 7 Feb 2024 09:38:54 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : from : subject : to : cc : references : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=AVBRuGVQI+WDA01AsklGk6zNsN0aZWQH57LpNfU3gXY=; b=IubP72IteVT232PQi9ySdJ8hr2gxZY9fZlbR6/BW0KUHnxWB1p2Kp7msqSV9LrrbKPw9 UutBX6oKaz7eSmSOoffYTsjHA2hDS81JwvufMrd4J56t+AcoSXYymjVUWpdpmptByqAb zxuCbSZHQpcVCNOO12LxG5rJfXEJcYkYhOjntS+xEHThyrA/1Q1mJ01x/AsNMfDRiUft VwE1zIZyXnAP7+aJUJIGNQVKr2cVxeEUz7w1C0NmXTcXvMo/JA/IXFHPWvq+pzCccNYu n8s64tLsT2YVvVGdBijwFKaRenJ2FnpXNu6fas7enQ+dClfekzN47BhuH0WuaQxZ1nWm /Q== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3w46vt8vc8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 07 Feb 2024 09:38:54 +0000 Received: from m0353725.ppops.net (m0353725.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 4179M47E002175; Wed, 7 Feb 2024 09:38:53 GMT Received: from ppma22.wdc07v.mail.ibm.com (5c.69.3da9.ip4.static.sl-reverse.com [169.61.105.92]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3w46vt8vbp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 07 Feb 2024 09:38:53 +0000 Received: from pps.filterd (ppma22.wdc07v.mail.ibm.com [127.0.0.1]) by ppma22.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 4176ifwP008752; Wed, 7 Feb 2024 09:38:53 GMT Received: from smtprelay06.fra02v.mail.ibm.com ([9.218.2.230]) by ppma22.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3w206yn1d2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 07 Feb 2024 09:38:53 +0000 Received: from smtpav01.fra02v.mail.ibm.com (smtpav01.fra02v.mail.ibm.com [10.20.54.100]) by smtprelay06.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 4179co0J38142536 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 7 Feb 2024 09:38:50 GMT Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E563920040; Wed, 7 Feb 2024 09:38:49 +0000 (GMT) Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D80FC20043; Wed, 7 Feb 2024 09:38:47 +0000 (GMT) Received: from [9.197.253.35] (unknown [9.197.253.35]) by smtpav01.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 7 Feb 2024 09:38:47 +0000 (GMT) Message-ID: <1235feaf-ecea-b18d-ce83-ec30eb842dfb@linux.ibm.com> Date: Wed, 7 Feb 2024 17:38:46 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 From: "Kewen.Lin" Subject: Re: Repost [PATCH 3/6] PowerPC: Add support for accumulators in DMR registers. To: Michael Meissner Cc: Segher Boessenkool , David Edelsohn , gcc-patches@gcc.gnu.org, Peter Bergner References: Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: sl3T5g3-IN_MxNW8eawu6NxOPGFMrWXO X-Proofpoint-GUID: 8t6L_bc60JJHmIczPqAt6hR0GcYQyif4 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-02-07_03,2024-01-31_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 lowpriorityscore=0 phishscore=0 spamscore=0 malwarescore=0 bulkscore=0 adultscore=0 mlxlogscore=999 priorityscore=1501 impostorscore=0 mlxscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000 definitions=main-2402070071 X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,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 List-Id: on 2024/2/7 08:06, Michael Meissner wrote: > On Thu, Jan 25, 2024 at 05:28:49PM +0800, Kewen.Lin wrote: >> Hi Mike, >> >> on 2024/1/6 07:38, Michael Meissner wrote: >>> The MMA subsystem added the notion of accumulator registers as an optional >>> feature of ISA 3.1 (power10). In ISA 3.1, these accumulators overlapped with >>> the traditional floating point registers 0..31, but logically the accumulator >>> registers were separate from the FPR registers. In ISA 3.1, it was anticipated >> >> Using VSX register 0..31 rather than traditional floating point registers 0..31 >> seems more clear, since floating point registers imply 64 bit long registers. > > Ok. > >>> that in future systems, the accumulator registers may no overlap with the FPR >>> registers. This patch adds the support for dense math registers as separate >>> registers. >>> >>> This particular patch does not change the MMA support to use the accumulators >>> within the dense math registers. This patch just adds the basic support for >>> having separate DMRs. The next patch will switch the MMA support to use the >>> accumulators if -mcpu=future is used. >>> >>> For testing purposes, I added an undocumented option '-mdense-math' to enable >>> or disable the dense math support. >> >> Can we avoid this and use one macro for it instead? As you might have noticed >> that some previous temporary options like -mpower{8,9}-vector cause ICEs due to >> some unexpected combination and we are going to neuter them, so let's try our >> best to avoid it if possible. I guess one macro TARGET_DENSE_MATH defined by >> TARGET_FUTURE && TARGET_MMA matches all use places? and specifying -mcpu=future >> can enable it while -mcpu=power10 can disable it. > > That depends on whether there will be other things added in the future power > that are not in the MMA+ instruction set. > > But I can switch to defining TARGET_DENSE_MATH to testing TARGET_FUTURE and > TARGET_MMA. That way if/when a new cpu comes out, we will just have to change > the definition of TARGET_DENSE_MATH and not all of the uses. Yes, that's what I expected. Thanks! > > I will also add TARGET_MMA_NO_DENSE_MATH to handle the existing MMA code for > assemble and disassemble when we don't have dense math instructions. Nice, I also found having such macro can help when reviewing one latter patch so suggested a similar there. >>> -(define_insn_and_split "*movxo" >>> +(define_insn_and_split "*movxo_nodm" >>> [(set (match_operand:XO 0 "nonimmediate_operand" "=d,ZwO,d") >>> (match_operand:XO 1 "input_operand" "ZwO,d,d"))] >>> - "TARGET_MMA >>> + "TARGET_MMA && !TARGET_DENSE_MATH >>> && (gpc_reg_operand (operands[0], XOmode) >>> || gpc_reg_operand (operands[1], XOmode))" >>> "@ >>> @@ -366,6 +369,31 @@ (define_insn_and_split "*movxo" >>> (set_attr "length" "*,*,16") >>> (set_attr "max_prefixed_insns" "2,2,*")]) >>> >>> +(define_insn_and_split "*movxo_dm" >>> + [(set (match_operand:XO 0 "nonimmediate_operand" "=wa,QwO,wa,wD,wD,wa") >>> + (match_operand:XO 1 "input_operand" "QwO,wa, wa,wa,wD,wD"))] >> >> Why not adopt ZwO rather than QwO? > > You have to split the address into 2 addresses for loading or storing vector > pairs (or 4 addresses for loading or storing vectors). Z would allow > register+register addresses, and you wouldn't be able to create the second > address by adding 128 to it. Hence it uses 'Q' for register only and 'wo' for > d-form addresses. Thanks for clarifying. But without this patch the define_insn_and_split *movxo adopts "ZwO", IMHO it would mean the current "*movxo" define_insn_and_split have been problematic? I thought adjust_address can ensure the new address would be still valid after adjusting 128 offset, could you double check? > >> >>> + "TARGET_DENSE_MATH >>> + && (gpc_reg_operand (operands[0], XOmode) >>> + || gpc_reg_operand (operands[1], XOmode))" >>> + "@ >>> + # >>> + # >>> + # >>> + dmxxinstdmr512 %0,%1,%Y1,0 >>> + dmmr %0,%1 >>> + dmxxextfdmr512 %0,%Y0,%1,0" >>> + "&& reload_completed >>> + && !dmr_operand (operands[0], XOmode) >>> + && !dmr_operand (operands[1], XOmode)" >>> + [(const_int 0)] >>> +{ >>> + rs6000_split_multireg_move (operands[0], operands[1]); >>> + DONE; >>> +} >>> + [(set_attr "type" "vecload,vecstore,veclogical,mma,mma,mma") >>> + (set_attr "length" "*,*,16,*,*,*") >>> + (set_attr "max_prefixed_insns" "2,2,*,*,*,*")]) >>> + ... >>> +;; Return 1 if op is a DMR register >>> +(define_predicate "dmr_operand" >>> + (match_operand 0 "register_operand") >>> +{ >>> + if (!REG_P (op)) >>> + return 0; >>> + >>> + if (!HARD_REGISTER_P (op)) >>> + return 1; >>> + >>> + return DMR_REGNO_P (REGNO (op)); >>> +}) >>> + >>> +;; Return 1 if op is an accumulator. On power10 systems, the accumulators >>> +;; overlap with the FPRs, while on systems with dense math, the accumulators >>> +;; are separate dense math registers and do not overlap with the FPR >>> +;; registers.. >> >> Nit: an unexpected "."? >> >>> +(define_predicate "accumulator_operand" >>> + (match_operand 0 "register_operand") >>> +{ >> >> fpr_reg_operand checks for subreg as well, should we check for it here as well? >> >>> #ifdef TARGET_REGNAMES >>> @@ -1250,6 +1255,8 @@ static const char alt_reg_names[][8] = >>> "%cr0", "%cr1", "%cr2", "%cr3", "%cr4", "%cr5", "%cr6", "%cr7", >>> /* vrsave vscr sfp */ >>> "vrsave", "vscr", "sfp", >>> + /* DMRs */ >>> + "%dmr0", "%dmr1", "%dmr2", "%dmr3", "%dmr4", "%dmr5", "%dmr6", "%dmr7", >> >> Should be without "r" here, as tested gas doesn't recognize %dmr0 but it does >> recognize %dm0. I guessed some reply was missing on this part (and some latter others)? Just want to ensure something wasn't missing and hope this helps. :) >> >>> }; >>> #endif >>> >>> @@ -1846,6 +1853,9 @@ rs6000_hard_regno_nregs_internal (int regno, machine_mode mode) >>> else if (ALTIVEC_REGNO_P (regno)) >>> reg_size = UNITS_PER_ALTIVEC_WORD; >>> >>> + else if (DMR_REGNO_P (regno)) >>> + reg_size = UNITS_PER_DMR_WORD; >>> + >>> else >>> reg_size = UNITS_PER_WORD; >>> >>> @@ -1867,9 +1877,36 @@ rs6000_hard_regno_mode_ok_uncached (int regno, machine_mode mode) >>> if (mode == OOmode) >>> return (TARGET_MMA && VSX_REGNO_P (regno) && (regno & 1) == 0); >>> >>> - /* MMA accumulator modes need FPR registers divisible by 4. */ >>> + /* On ISA 3.1 (power10), MMA accumulator modes need FPR registers divisible >>> + by 4. >>> + >>> + If dense math is enabled, allow all VSX registers plus the DMR registers. >>> + We need to make sure we don't cross between the boundary of FPRs and >>> + traditional Altiviec registers. */ >>> if (mode == XOmode) >>> - return (TARGET_MMA && FP_REGNO_P (regno) && (regno & 3) == 0); >>> + { >>> + if (TARGET_MMA && !TARGET_DENSE_MATH) >>> + return (FP_REGNO_P (regno) && (regno & 3) == 0); >>> + >>> + else if (TARGET_DENSE_MATH) >>> + { >>> + if (DMR_REGNO_P (regno)) >>> + return 1; >>> + >>> + if (FP_REGNO_P (regno)) >>> + return ((regno & 1) == 0 && regno <= LAST_FPR_REGNO - 3); >>> + >>> + if (ALTIVEC_REGNO_P (regno)) >>> + return ((regno & 1) == 0 && regno <= LAST_ALTIVEC_REGNO - 3); >>> + } >> >> I could miss something, I didn't find which section of RFC indicates this >> restriction, could you please point out for me? Thanks! >> >>> + >>> + else >>> + return 0; >>> + } >>> + >>> + /* No other types other than XOmode can go in DMRs. */ >>> + if (DMR_REGNO_P (regno)) >>> + return 0; >>> >>> /* PTImode can only go in GPRs. Quad word memory operations require even/odd >>> register combinations, and use PTImode where we need to deal with quad >>> @@ -2312,6 +2349,7 @@ rs6000_debug_reg_global (void) >>> rs6000_debug_reg_print (FIRST_ALTIVEC_REGNO, >>> LAST_ALTIVEC_REGNO, >>> "vs"); >>> + rs6000_debug_reg_print (FIRST_DMR_REGNO, LAST_DMR_REGNO, "dmr"); >> >> Nit: Like above, use 'dm'. >> >>> rs6000_debug_reg_print (LR_REGNO, LR_REGNO, "lr"); >>> rs6000_debug_reg_print (CTR_REGNO, CTR_REGNO, "ctr"); >>> rs6000_debug_reg_print (CR0_REGNO, CR7_REGNO, "cr"); >>> @@ -2332,6 +2370,7 @@ rs6000_debug_reg_global (void) >>> "wr reg_class = %s\n" >>> "wx reg_class = %s\n" >>> "wA reg_class = %s\n" >>> + "wD reg_class = %s\n" >>> "\n", >>> reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_d]], >>> reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_v]], >>> @@ -2339,7 +2378,8 @@ rs6000_debug_reg_global (void) >>> reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_we]], >>> reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wr]], >>> reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wx]], >>> - reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wA]]); >>> + reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wA]], >>> + reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wD]]); >>> >> >> snip ... >> >>> +/* Subroutine to determine the move cost of dense math registers. If we are >>> + moving to/from VSX_REGISTER registers, the cost is either 1 move (for >>> + 512-bit accumulators) or 2 moves (for 1,024 dmr registers). If we are >>> + moving to anything else like GPR registers, make the cost very high. */ >>> + >>> +static int >>> +rs6000_dmr_register_move_cost (machine_mode mode, reg_class_t rclass) >>> +{ >>> + const int reg_move_base = 2; >>> + HARD_REG_SET vsx_set = (reg_class_contents[rclass] >>> + & reg_class_contents[VSX_REGS]); >>> + >>> + if (TARGET_DENSE_MATH && !hard_reg_set_empty_p (vsx_set)) >> >> Can we just use reg_classes_intersect_p (rclass, VSX_REGS)? >> BR, Kewen