From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2a07:de40:b251:101:10:150:64:2]) by sourceware.org (Postfix) with ESMTPS id 5DAC63858D1E for ; Wed, 29 Nov 2023 07:20:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5DAC63858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5DAC63858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:2 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701242420; cv=none; b=fImD9EYew4q3kINBg/oenBpUVkTcnv7rJ0tKC9s10Yn4/wee4IgzssT1m4ZgWSaR77mpIsHjao8f2Jhe0h/wR/z6Ubsk4QLuk79nlexY+G0Uh/FLhteaugMdFYEtxbOArKJVdIBX/uRx+HJ8wDhVctUcmBxrNm8sRvwKA3NOuVo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701242420; c=relaxed/simple; bh=msP/8T04yQDOCgoV4OL4AAVIvoP7H2gU/GeFufHq1pE=; h=Date:From:To:Subject:Message-ID:MIME-Version; b=P1ygTVDZA0EVzB94M6KaJAJ4iF16zw+OkkfX+xsZhr4QZH/VLKfwpBLBdjElhI5vka7t/XPMw66npzIyoOSuj0o5bb+CcMiLR0O+1ar1VAmJZnkZ6V1HNQ5XLqG2J8oCa4KCi25XZx9slcLFAUjjmV874CLnhUDuvAiRSbfAYAA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from relay2.suse.de (unknown [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 5FBB91F88F; Wed, 29 Nov 2023 07:20:17 +0000 (UTC) Received: from [10.168.4.150] (unknown [10.168.4.150]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B3E3F2C15C; Wed, 29 Nov 2023 07:20:15 +0000 (UTC) Date: Wed, 29 Nov 2023 08:16:38 +0100 (CET) From: Richard Biener To: Jeff Law cc: Robin Dapp , gcc-patches@gcc.gnu.org, "Li, Pan2" Subject: Re: [PATCH][RFC] middle-end/110237 - wrong MEM_ATTRs for partial loads/stores In-Reply-To: Message-ID: References: <20230621075019.7CA813858033@sourceware.org> <5b04dad7-a241-4ad3-b4a5-b81080b67e90@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Level: X-Rspamd-Server: rspamd1 Authentication-Results: smtp-out2.suse.de; none X-Rspamd-Queue-Id: 5FBB91F88F X-Spam-Score: -4.00 X-Spamd-Result: default: False [-4.00 / 50.00]; REPLY(-4.00)[]; TAGGED_RCPT(0.00)[] X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,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, 28 Nov 2023, Jeff Law wrote: > > > On 11/28/23 00:50, Richard Biener wrote: > > > > > There's no way to distinguish a partial vs. non-partial MEM on RTL and > > while without the bogus MEM_ATTR the alias oracle pieces that > > miscompiled the original case are fended off we still see the load/store > > as full given they have a mode with a size - that for example means > > that DSE can elide a previous store to a masked part. Eventually > > that's fended off by using an UNSPEC, but whether the RTL IL has > > the correct semantics is questionable. > > > > That said, I did propose scrapping the MEM_EXPR which I think is > > the correct thing to do unless we want to put a CALL_EXPR into it > > (nothing would use that at the moment) or re-do MEM_EXPR and instead > > have an ao_ref (or sth slightly more complete) instead of the current > > MEM_ATTRs - but that would be a lot of work. > > > > This leaves the question wrt. semantics of for example x86 mask_store: > > > > (insn 23 22 24 5 (set (mem:V4DF (plus:DI (reg/v/f:DI 106 [ x ]) > > (reg:DI 101 [ ivtmp.15 ])) [2 MEM > > [(double *)x_11(D) + ivtmp.15_33 * 1]+0 S32 A64]) > > (unspec:V4DF [ > > (reg:V4DI 104 [ mask__16.8 ]) > > (reg:V4DF 105 [ vect_cst__42 ]) > > (mem:V4DF (plus:DI (reg/v/f:DI 106 [ x ]) > > (reg:DI 101 [ ivtmp.15 ])) [2 MEM > double> [(double *)x_11(D) + ivtmp.15_33 * 1]+0 S32 A64]) > > ] UNSPEC_MASKMOV)) "t.c":5:12 8523 {avx_maskstorepd256} > > (nil)) > > > > it uses a read-modify-write which makes it safe for DSE. > Agreed. > > > mask_load > > looks like > > > > (insn 28 27 29 6 (set (reg:V4DF 115 [ vect__7.11 ]) > > (unspec:V4DF [ > > (reg:V4DI 114 [ mask__8.8 ]) > > (mem:V4DF (plus:DI (reg/v/f:DI 118 [ val ]) > > (reg:DI 103 [ ivtmp.29 ])) [2 MEM > double> [(double *)val_13(D) + ivtmp.29_22 * 1]+0 S32 A64]) > > ] UNSPEC_MASKMOV)) "t.c":5:17 8515 {avx_maskloadpd256} > > (nil)) > So with the mem:V4DF inside the unspec, ISTM we must treat that as a potential > full read, but we can't rely on it being a full read. I don't think UNSPEC > semantics are that it must read/consume all its operands in full, just that it > might. That might be worth a documentation clarification. > > > > > > both have (as operand of the UNSPEC) a MEM with V4DFmode (and a > > MEM_EXPR with a similarly bougs MEM_EXPR) indicating the loads > > are _not_ partial. That means the disambiguation against a store > > to an object that's smaller than V4DF is still possible. > > Setting MEM_SIZE to UNKNOWN doesn't help - that just asks to look > > at the mode. As discussed using a BLKmode MEM _might_ be a way > > out but I didn't try what will happen then (patterns would need to > > be adjusted I guess). > > > > That said, I'm happy to commit the partial fix, scrapping the > > bogus MEM_EXPRs. > > > > OK for that? > Works for me. I'm re-testing the change and will push. If the UNSPEC uses are really OK I think we're set. We can incrementally try to restore missing alias info. Richard. > jeff > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)