From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id DEEDE385DDFC for ; Tue, 11 Jun 2024 21:19:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DEEDE385DDFC Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DEEDE385DDFC Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718140793; cv=none; b=Ez7c5kq1yCPidwbV2unz2PTeORjrscNO/iQ7+kkV86vROpJiezuVAZuzcR+sGlgqSY+uD23FDvdx0rIHgwkZCv9p48Bku6LuV04iuKfj7TlxHZRRx4mspkC/VM5MzsaNItrHWcj5t16DurwYVqMt7sbSbAReiLQPy4Q3CtKAvbA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718140793; c=relaxed/simple; bh=PjRhNkHy+3Uy8mur97crKbdczgn1m5irrmieqaLnV8Y=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=nUwPTNgisQzjgkuvDpbEeHy6B6IcMhlyjOw8A1Y/zdguYnpH3bLGE6eDyJ0bVW7wZD0bmlsxtk2dIQ9FuNx/BKWNL9p2HE/F+3/z65BrDQy7/zvtDMdRr3J2gHJX/HOs2SYrIrMx9QttukSLQJe3vGeVyL41BjQob2ozX5SskHE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 02DB0152B; Tue, 11 Jun 2024 14:20:16 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B04483F73B; Tue, 11 Jun 2024 14:19:50 -0700 (PDT) From: Richard Sandiford To: Robin Dapp Mail-Followup-To: Robin Dapp ,gcc-patches , jeffreyalaw , Manolis Tsamis , richard.sandiford@arm.com Cc: gcc-patches , jeffreyalaw , Manolis Tsamis Subject: Re: [PATCH] ifcvt: Clarify if_info.original_cost. References: <57bb6ce5-79c3-4b08-b524-e807b9ac431b@gmail.com> <310545d3-b438-4f70-af0a-a6433614dadf@gmail.com> Date: Tue, 11 Jun 2024 22:19:49 +0100 In-Reply-To: <310545d3-b438-4f70-af0a-a6433614dadf@gmail.com> (Robin Dapp's message of "Tue, 11 Jun 2024 21:32:11 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,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 List-Id: Robin Dapp writes: >> I was looking at the code in more detail and just wanted to check. >> We have: >> >> int last_needs_comparison = -1; >> >> bool ok = noce_convert_multiple_sets_1 >> (if_info, &need_no_cmov, &rewired_src, &targets, &temporaries, >> &unmodified_insns, &last_needs_comparison); >> if (!ok) >> return false; >> >> /* If there are insns that overwrite part of the initial >> comparison, we can still omit creating temporaries for >> the last of them. >> As the second try will always create a less expensive, >> valid sequence, we do not need to compare and can discard >> the first one. */ >> if (last_needs_comparison != -1) >> { >> end_sequence (); >> start_sequence (); >> ok = noce_convert_multiple_sets_1 >> (if_info, &need_no_cmov, &rewired_src, &targets, &temporaries, >> &unmodified_insns, &last_needs_comparison); >> /* Actually we should not fail anymore if we reached here, >> but better still check. */ >> if (!ok) >> return false; >> } >> >> But noce_convert_multiple_sets_1 ends with: >> >> /* Even if we did not actually need the comparison, we want to make sure >> to try a second time in order to get rid of the temporaries. */ >> if (*last_needs_comparison == -1) >> *last_needs_comparison = 0; >> >> >> return true; >> >> AFAICT that means that the first attempt is always redundant. >> >> Have I missed something? > > (I might not have fully gotten the question) > > The idea is that the first attempt goes through all insns and sets > *last_need_comparison to the insn number that either > - used the condition/comparison by preferring seq1 or > - used the condition as a side-effect insn when creating a CC-using > insn in seq2. > (And we only know that after actually creating the sequences). > > The second attempt then improves on the first one by skipping > any temporary destination registers after the last insn that required > the condition (even though its target overlaps with the condition > registers). This is true for all cmovs that only use the CC > (instead of the condition). Essentially, we know that all following > cmovs can be created via the CC which is not overwritten. > > So, even when we never used the condition because of all CC-using > cmovs we would skip the temporary targets in the second attempt. > But we can't know that all we ever needed is the CC comparison > before actually creating the sequences in the first attempt. Hmm, ok. The bit that confused me most was: if (last_needs_comparison != -1) { end_sequence (); start_sequence (); ... } which implied that the second attempt was made conditionally. It seems like it's always used and is an inherent part of the algorithm. If the problem is tracking liveness, wouldn't it be better to iterate over the "then" block in reverse order? We would start with the liveness set for the join block and update as we move backwards through the "then" block. This liveness set would tell us whether the current instruction needs to preserve a particular register. That should make it possible to do the transformation in one step, and so avoid the risk that the second attempt does something that is unexpectedly different from the first attempt. FWIW, the reason for asking was that it seemed safer to pass use_cond_earliest back from noce_convert_multiple_sets_1 to noce_convert_multiple_sets, as another parameter, and then do the adjustment around noce_convert_multiple_sets's call to targetm.noce_conversion_profitable_p. That would avoid the new for a new if_info field, which in turn would make it less likely that stale information is carried over from one attempt to the next (e.g. if other ifcvt techniques end up using the same field in future). Thanks, Richard