From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id E41293858C2D for ; Tue, 27 Sep 2022 12:34:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E41293858C2D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id B5B4021D7F; Tue, 27 Sep 2022 12:34:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1664282052; h=from:from:reply-to: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=lm8I9YThOEcnBD4F/FWggIbY9VOC0beYYg88RJPsK0Q=; b=SRS5K9TT9i5JlxOjPosLqu1JaA6niamgQ1JM5i50YQufHZEZjvI2uG7brJ657OCc1e5PqG qw9aS7n2bOMb2r1/47RSPE51RD/kybRnLIc4v47i50tN9HBNsOfWMZfeCBjMVcpMOCZECh w9lDlGfRy9hYIHht+1BHcPsFjt9iBeY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1664282052; h=from:from:reply-to: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=lm8I9YThOEcnBD4F/FWggIbY9VOC0beYYg88RJPsK0Q=; b=b49vdinACEyca0B6ph/6VSjDss0YuXbj6OJb1IHcyAcUKaF6Kmo/uaLfCIitT9PYr0o68g f63prsoyQYKn0lCw== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (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 8DB822C165; Tue, 27 Sep 2022 12:34:12 +0000 (UTC) Date: Tue, 27 Sep 2022 12:34:12 +0000 (UTC) From: Richard Biener To: "Andre Vieira (lists)" cc: Jakub Jelinek , Richard Sandiford , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] Teach vectorizer to deal with bitfield accesses (was: [RFC] Teach vectorizer to deal with bitfield reads) In-Reply-To: <5c3e3af5-c502-fd5a-2792-4e0d1db405ef@arm.com> Message-ID: References: <4a6f2350-f070-1473-63a5-3232968d3a89@arm.com> <4ea91767-de43-9d30-5bd2-a9a0759760c7@arm.com> <69abe824-94f5-95b5-fb7f-6fa076973e05@arm.com> <8f805fb1-d4ae-b0e3-ff26-57fd2c1fc1f7@arm.com> <5c3e3af5-c502-fd5a-2792-4e0d1db405ef@arm.com> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,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 Mon, 26 Sep 2022, Andre Vieira (lists) wrote: > > On 08/09/2022 12:51, Richard Biener wrote: > > > > I'm curious, why the push to redundant_ssa_names? That could use > > a comment ... > So I purposefully left a #if 0 #else #endif in there so you can see the two > options. But the reason I used redundant_ssa_names is because ifcvt seems to > use that as a container for all pairs of (old, new) ssa names to replace > later. So I just piggy backed on that. I don't know if there's a specific > reason they do the replacement at the end? Maybe some ordering issue? Either > way both adding it to redundant_ssa_names or doing the replacement inline work > for the bitfield lowering (or work in my testing at least). Possibly because we (in the past?) inserted/copied stuff based on predicates generated at analysis time after we decide to elide something so we need to watch for later appearing uses. But who knows ... my mind fails me here. If it works to replace uses immediately please do so. But now I wonder why we need this - the value shouldn't change so you should get away with re-using the existing SSA name for the final value? > > Note I fear we will have endianess issues when translating > > bit-field accesses to BIT_FIELD_REF/INSERT and then to shifts. Rules > > for memory and register operations do not match up (IIRC, I repeatedly > > run into issues here myself). The testcases all look like they > > won't catch this - I think an example would be sth like > > struct X { unsigned a : 23; unsigned b : 9; }, can you see to do > > testing on a big-endian target? > I've done some testing and you were right, it did fall apart on big-endian. I > fixed it by changing the way we compute the 'shift' value and added two extra > testcases for read and write each. > > > > Sorry for the delay in reviewing. > No worries, apologies myself for the delay in reworking this, had a nice > little week holiday in between :) > > I'll write the ChangeLogs once the patch has stabilized. Thanks, Richard.