From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 8AE7C3858C2F for ; Mon, 15 Aug 2022 21:31:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8AE7C3858C2F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 27FLU8Mq008102; Mon, 15 Aug 2022 16:30:08 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 27FLU7lD008098; Mon, 15 Aug 2022 16:30:07 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 15 Aug 2022 16:30:06 -0500 From: Segher Boessenkool To: "Kewen.Lin" Cc: Peter Bergner , GCC Patches , David Edelsohn , will schmidt Subject: Re: [PATCH v3] rs6000: Adjust mov optabs for opaque modes [PR103353] Message-ID: <20220815213006.GK25951@gate.crashing.org> References: <09c34b29-feea-d26e-2c4f-5e096ab286bc@linux.ibm.com> <20220623190619.GU25951@gate.crashing.org> <6ded988f-6684-e42e-ca82-d81ff55178d2@linux.ibm.com> <20220624164917.GX25951@gate.crashing.org> <1647e4bb-fa61-065f-d90d-b56503f44770@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1647e4bb-fa61-065f-d90d-b56503f44770@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2022 21:31:12 -0000 Hi! On Mon, Jun 27, 2022 at 10:47:26AM +0800, Kewen.Lin wrote: > on 2022/6/25 00:49, Segher Boessenkool wrote: > Many thanks for all the further explanation above! The attached patch > updated the misleading comments as you pointed out and suggested, could > you help to have another look? Please do not send proposed patches in the middle of a mail thread. It is harder to track things than needed this way, and it makes it hard to apply your patches as well (for test builds, say). > Subject: [PATCH] rs6000: Adjust mov optabs for opaque modes [PR103353] > > As PR103353 shows, we may want to continue to expand built-in > function __builtin_vsx_lxvp, even if we have already emitted > error messages about some missing required conditions. As > shown in that PR, without one explicit mov optab on OOmode > provided, it would call emit_move_insn recursively. > - rs6000_emit_move (operands[0], operands[1], OOmode); > - DONE; > + if (TARGET_MMA) { > + rs6000_emit_move (operands[0], operands[1], OOmode); > + DONE; > + } if (TARGET_MMA) { rs6000_emit_move (operands[0], operands[1], OOmode); DONE; } > + /* PR103353 shows we may want to continue to expand the __builtin_vsx_lxvp > + built-in function, even if we have already emitted error messages about > + some missing required conditions. As shown in that PR, without one > + explicit mov optab on OOmode provided, it would call emit_move_insn > + recursively. So we allow this pattern to be generated when we are > + expanding to RTL and have seen errors. It would not cause further ICEs > + as the compilation would stop soon after expanding. */ > + else if (currently_expanding_to_rtl && seen_error ()) > + ; The comment goes inside this "else if" branch. Maybe make it a braced block if the semicolon looks out of place without it. Removing the TARGET_MMA requirement is the correct thing to do no matter what: this is not an MMA insn at all after all, and neither is it only useful if MMA is enabled, etc. Is the later "unreachable" ever useful? If not, you could just fall through after that "if (TARGET_MMA)" thing. Worst that will happen you get an OO move in the insn stream that we will error on later :-) The patch is okay for trunk with the indentation fixed. Thanks! Segher