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 78E883858D1E for ; Fri, 2 Sep 2022 17:51:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 78E883858D1E 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 (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 282HjTEg014228; Fri, 2 Sep 2022 17:51:37 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=pp1; bh=NxTMC0qmEnBcAFHdrnEzY8Xg5IPy/2iHYNUvdq9HfmA=; b=fDLgn3pMUKkqW8nsYKWYf748uq3gtIMRlKyu+jh29vmoPdOkyl15irBtNBu06INDnvo2 QjJT2TiC5rq+Osg+Q+yhGrqkXJsp4WXQARjkTIeX+78YJRRbxikaHUFMQgiq03Mx+vRv yGTbI1OT80wDHksZnnV0uRrRtbaKEfSgBl5dkhDNtxyOmhvHYp+9hhxsDAvjkjsQan8O AjSrBAsWWm+60X/1JvOEZngd1MRc0LqTsgHy4PGYwb5sx5EUZn4q+g4ZmuV3k3X3MhPR ogHEcyTrbF/BUB62Yy3Qn06RMsw2pZ7ffiTXn7YWsTgvimnwSi1/KFUgAAjL0iRscPLs Jg== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3jbpgwg4vd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 02 Sep 2022 17:51:37 +0000 Received: from m0098410.ppops.net (m0098410.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 282HjZ3h014573; Fri, 2 Sep 2022 17:51:37 GMT Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3jbpgwg4v1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 02 Sep 2022 17:51:37 +0000 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 282HLWmw001604; Fri, 2 Sep 2022 17:51:36 GMT Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by ppma04dal.us.ibm.com with ESMTP id 3j7awakqnf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 02 Sep 2022 17:51:36 +0000 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 282HpZZ563701470 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 2 Sep 2022 17:51:35 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3E9D1124052; Fri, 2 Sep 2022 17:51:35 +0000 (GMT) Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E61CF124054; Fri, 2 Sep 2022 17:51:34 +0000 (GMT) Received: from toto.the-meissners.org (unknown [9.160.69.146]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTPS; Fri, 2 Sep 2022 17:51:34 +0000 (GMT) Date: Fri, 2 Sep 2022 13:51:33 -0400 From: Michael Meissner To: Segher Boessenkool Cc: Michael Meissner , gcc-patches@gcc.gnu.org, "Kewen.Lin" , David Edelsohn , Peter Bergner , Will Schmidt Subject: Re: [PATCH] Improve converting between 128-bit modes that use the same format Message-ID: Mail-Followup-To: Michael Meissner , Segher Boessenkool , gcc-patches@gcc.gnu.org, "Kewen.Lin" , David Edelsohn , Peter Bergner , Will Schmidt References: <20220823211345.GW25951@gate.crashing.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220823211345.GW25951@gate.crashing.org> X-TM-AS-GCONF: 00 X-Proofpoint-GUID: wD5TTAEWDZxzJouEyN7Y1guB4zz5CW6r X-Proofpoint-ORIG-GUID: uoSDmzUupBrbd28tsptI82iQRC_NUDl9 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-09-02_04,2022-08-31_03,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 mlxlogscore=999 lowpriorityscore=0 spamscore=0 suspectscore=0 impostorscore=0 bulkscore=0 clxscore=1015 priorityscore=1501 phishscore=0 malwarescore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2209020081 X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_MSPIKE_H2,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 Tue, Aug 23, 2022 at 04:13:45PM -0500, Segher Boessenkool wrote: > Please do not send new patches as replies to other patches. This was sent as a new patch. > On Thu, Aug 18, 2022 at 05:48:29PM -0400, Michael Meissner wrote: > > mprove converting between 128-bit modes that use the same format. > > You are missing some characters? But this is an edited version of the > subject anyway. Just don't do that (neither the copying or the > editing), it just confuses things. That is the first line from the git commit, which git format-patch puts as the subject. I accidently deleted a few extra characters when trimming it down (I remove the From:, etc. lines from the format-patch output). But I can just delete this line if desired. > Please factor this patch into more pieces, pieces that can be reviewed > more easily, pieces that change one thing only. > > As is you are just rewriting the lot, and it is not an improvement at > all this way. No doubt there are many good pieces in it, but mixed with > a non-trivial amount of bad pieces I cannot approve it. It also isn't > clear at all what you want to do; piece by piece it is easier to > explain. > > > -; Iterators for converting to/from TFmode > > -(define_mode_iterator IFKF [IF KF]) > > Yes, IFmode and KFmode have almost nothing in common. Good to see this > go. It would be even better if we would not use > rs6000_expand_float128_convert when not needed, either, and all this > would be just gone after expand. I took a look at it, and I have a new version that only does the moves that are NOPs, and it makes sure all of the functions called have the proper names. I will post it on Tuesday, as some of the machines that I use for testing are now down for the US Labor Day weekend (they need to work on power infrastructure to the lab the machines are in). > > > +(define_expand "extendkfif2" > > + [(set (match_operand:IF 0 "gpc_reg_operand") > > + (float_extend:IF (match_operand:KF 1 "gpc_reg_operand")))] > > + "TARGET_FLOAT128_TYPE" > > +{ > > + rs6000_expand_float128_convert (operands[0], operands[1], false); > > + DONE; > > +}) > > This does not belong here. > > It really shouldn't *exist* at all: this is *not* a float_extend! It is > not converting to a wider mode (as required!), but not even to a mode of > higher precision: both IFmode and KFmode can represent (finite, normal) > numbers the other can not. We know that TFmode (if -mabi=ieeelongdouble) and KFmode are the same, just like TFmode (if -mabi=ibmlongdouble) and IFmode are the same. But RTL does not know that these modes use the same representation. So to convert between them, it needs to use either FLOAT_EXTEND or FLOAT_TRUNCATE, depending on which precision each of the three modes have (i.e. rs6000-modes.h). So you need these conversions in RTL. Unfortunately, you can't just use SUBREG before register allocation is done. So I do define_insn_and_split to cover this. > > But it certainly does not belong here in the middle of no-op moves. > > > +(define_expand "trunckfif2" > > + [(set (match_operand:IF 0 "gpc_reg_operand") > > + (float_truncate:IF (match_operand:KF 1 "gpc_reg_operand")))] > > + "TARGET_FLOAT128_TYPE" > > +{ > > + rs6000_expand_float128_convert (operands[0], operands[1], false); > > + DONE; > > +}) > > I also would expect IBM128 instead of just IF. This would simplify a > lot. Why do you not use that, is there a reason? If you use IBM128, you then need to create a mode_attr that for a given mode gives the other mode. Sure it can be done, but for the insns involved it was just simpler to duplicate the insns. So for example, for IBM floating point my current patches are: (define_insn_and_split "extendtfif2" [(set (match_operand:IF 0 "gpc_reg_operand" "=wa,wa,r,r") (float_extend:IF (match_operand:TF 1 "gpc_reg_operand" "0,wa,0,r")))] "TARGET_HARD_FLOAT && TARGET_IBM128 && FLOAT128_IBM_P (TFmode)" "#" "&& reload_completed" [(set (match_dup 0) (match_dup 2))] { operands[2] = gen_lowpart (IFmode, operands[1]); } [(set_attr "num_insns" "2") (set_attr "length" "8")]) (define_insn_and_split "extendiftf2" [(set (match_operand:TF 0 "gpc_reg_operand" "=wa,wa,r,r") (float_extend:TF (match_operand:IF 1 "gpc_reg_operand" "0,wa,0,r")))] "TARGET_HARD_FLOAT && TARGET_IBM128 && FLOAT128_IBM_P (TFmode)" "#" "&& reload_completed" [(set (match_dup 0) (match_dup 2))] { operands[2] = gen_lowpart (TFmode, operands[1]); } [(set_attr "num_insns" "2") (set_attr "length" "8")]) You could rewrite that as: (define_mode_attr IBM128_other [(IF "TF") ("TF" "IF")]) (define_insn_and_split "extend2" [(set (match_operand:IBM128 0 "gpc_reg_operand" "=wa,wa,r,r") (float_extend:IBM128 (match_operand:IBM128_othr 1 "gpc_reg_operand" "0,wa,0,r")))] "TARGET_HARD_FLOAT && TARGET_IBM128 && FLOAT128_IBM_P (TFmode)" "#" "&& reload_completed" [(set (match_dup 0) (match_dup 2))] { operands[2] = gen_lowpart (TFmode, operands[1]); } [(set_attr "num_insns" "2") (set_attr "length" "8")]) But for the IEEE side, combining the two insns won't work, since going from TFmode to KFmode will generate a FLOAT_TRUNCATE instead of a FLOAT_EXTEND. You could then use a code iterator for both FLOAT_TRUNCATE and FLOAT_EXTEND. Recognizing both forms might protect us in case some day somebody reorders the precision within rs6000-modes.h. But that is a lot of work to do for minimal gain. But I'm not convinced that it improves things. > > +;; Convert between KFmode and TFmode when -mabi=ieeelongdouble > > +(define_insn_and_split "*extendkftf2_internal" > > Same for IEEE128. And this isn't a conversion at all (it's a no-op > move), please don't confuse things by saying it is. That is how RTL goes between modes. Without a whole lot of changes to the machine independent portion of the compiler, I don't see anyway of avoiding the converts. You might say well always just use one mode for IEEE and one mode for IBM. I've tried, and I couldn't get it to work. Besides if we do this, we break people using __attribute__((mode(...))) which in turn is used by glibc and libstdc++ to get _Complex forms of the __float128 type. > > > + [(set_attr "type" "two") > > + (set_attr "num_insns" "2")]) > > Btw, that really should never be needed. insn_type "two" already means > exactly that. > > > +(define_insn_and_split "*extendtfif2_internal" > > + [(set (match_operand:IF 0 "gpc_reg_operand" "=d,&d") > > + (float_extend:IF > > + (match_operand:TF 1 "input_operand" "0,d")))] > > + "FLOAT128_IBM_P (TFmode)" > > + "#" > > + "&& reload_completed" > > Why would this ever need reload_completed? It is a no-op move! Various predicates do not allow SUBREG's of these types before register allocation. -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meissner@linux.ibm.com