From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 7F5393858D32 for ; Thu, 6 Apr 2023 10:31:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7F5393858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680777106; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=09e2AfEHuA6mvUnE2bidUskVPT6FUiQZElknpbVsDpY=; b=hC5u89an0DeX7ptnO8+DTB6my/qarBXaLD3/VYP3ntdT8ayvK67vLmC/a6rilDmtPF6asH FmuNERPiY58vcilbyqyqIUtGJyGFQ1oLqtnVzvmXLyfvJ26v/+KQV6CBZ4E4GcJhLDUNyH i2lhSHOTyP+WVfvl7+gav6fbRM5bQFs= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-529-36ESE4vpNZmiZG27EVOX4A-1; Thu, 06 Apr 2023 06:31:42 -0400 X-MC-Unique: 36ESE4vpNZmiZG27EVOX4A-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5159B101A553; Thu, 6 Apr 2023 10:31:42 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EBECF2166B26; Thu, 6 Apr 2023 10:31:41 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 336AVc6i3994361 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 6 Apr 2023 12:31:39 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 336AVblo3994359; Thu, 6 Apr 2023 12:31:37 +0200 Date: Thu, 6 Apr 2023 12:31:37 +0200 From: Jakub Jelinek To: Eric Botcazou , Jeff Law Cc: Richard Biener , Richard Sandiford , gcc-patches@gcc.gnu.org Subject: [PATCH] combine: Fix simplify_comparison AND handling for WORD_REGISTER_OPERATIONS targets [PR109040] Message-ID: Reply-To: Jakub Jelinek References: <612b6215-8bc3-1174-a475-4315176bfe1c@gmail.com> <2876279.e9J7NaK4W3@fomalhaut> MIME-Version: 1.0 In-Reply-To: <2876279.e9J7NaK4W3@fomalhaut> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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: On Thu, Apr 06, 2023 at 12:15:51PM +0200, Eric Botcazou wrote: > > Originally I didn't really see this as an operation. But the more and > > more I ponder it feels like it's an operation and thus should be subject > > to WORD_REGISTER_OPERATIONS. > > > > While it's not really binding on RTL semantics, if we look at how some > > architectures implement reg->reg copies, they're actually implemented > > with an ADD or IOR -- so a real operation under the hood. > > > > If we accept a subreg copy as an operation and thus subject to > > WORD_REGISTER_OPERATIONS then that would seem to imply the combine is > > the real problem here. Otherwise dse is the culprit. > > Yes, I agree that there is an ambiguity for subreg copy operations. At some > point I tried to define what register operations are and added a predicate to > that effect (word_register_operation_p ); while it returns true for SUBREG, > it's an opt-out predicate so this does not mean much. > > I don't think that DSE does anything wrong: as I wrote in the PR, defining > WORD_REGISTER_OPERATIONS should not prevent any particular form of RTL. > > I therefore think that the problem is in the combiner and probably in the > intermediate step shown by Jakub: > > "Then after that try_combine we do: > 13325 record_value_for_reg (dest, record_dead_insn, > 13326 WORD_REGISTER_OPERATIONS > 13327 && word_register_operation_p (SET_SRC > (setter)) > 13328 && paradoxical_subreg_p (SET_DEST > (setter)) > 13329 ? SET_SRC (setter) > 13330 : gen_lowpart (GET_MODE (dest), > 13331 SET_SRC (setter))); > and the 3 conditions are true here and so record value of the whole setter. > That then records among other things nonzero_bits as 0x8084c." > > That's a recent addition of mine (ae20d760b1ed69f631c3bf9351bf7e5005d52297) > and I think that it probably abuses WORD_REGISTER_OPERATIONS and should either > be reverted or restricted to the load case documented in its comment. I can > provide testing on SPARC if need be. If we want to fix it in the combiner, I think the fix would be following. The optimization is about (and:SI (subreg:SI (reg:HI xxx) 0) (const_int 0x84c)) and IMHO we can only optimize it into (subreg:SI (and:HI (reg:HI xxx) (const_int 0x84c)) 0) if we know that the upper bits of the REG are zeros. The optimization is only when the constant second AND operand is the same in both modes. Otherwise because of nonzero_bits ((reg:SI xxx), SImode) == 0x8084c we optimize into (subreg:SI (reg:HI xxx) 0). The original form has the upper 16 bits guaranteed to be all zeros, while the new form has them whatever value comes from the original operation. For !WORD_REGISTER_OPERATIONS, we don't optimize this way paradoxical SUBREGs at all, then by definition the upper bits are unspecified. Now, this patch fixes the PR, but certainly generates worse (but correct) code than the dse.cc patch. So perhaps we want both of them? As before, I unfortunately can't test it on riscv-linux (could perhaps try that on sparc-solaris on GCC Farm which is another WORD_REGISTER_OPERATIONS target, but last my bootstrap attempt there failed miserably because of the Don't bootstrap at midnight issue in cp/Make-lang.in; I'll post a patch for that once I test it). 2023-04-06 Jakub Jelinek PR target/109040 * combine.cc (simplify_comparison) : For swapping AND and SUBREG with paradoxical subreg on WORD_REGISTER_OPERATIONS target, ensure the upper bits are known to be 0. --- gcc/combine.cc.jj 2023-02-28 11:28:56.423182357 +0100 +++ gcc/combine.cc 2023-04-06 12:02:31.694747361 +0200 @@ -12671,7 +12671,11 @@ simplify_comparison (enum rtx_code code, && ((WORD_REGISTER_OPERATIONS && mode_width > GET_MODE_PRECISION (tmode) && mode_width <= BITS_PER_WORD - && trunc_int_for_mode (c1, tmode) == (HOST_WIDE_INT) c1) + && trunc_int_for_mode (c1, tmode) == (HOST_WIDE_INT) c1 + /* Make sure the bits above tmode in the SUBREG_REG + are all known to be 0, see PR109040. */ + && (nonzero_bits (SUBREG_REG (XEXP (op0, 0)), mode) + & ~GET_MODE_MASK (tmode)) == 0) || (mode_width <= GET_MODE_PRECISION (tmode) && subreg_lowpart_p (XEXP (op0, 0)))) && mode_width <= HOST_BITS_PER_WIDE_INT Jakub