From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id BB56F3858C53; Fri, 2 Jun 2023 10:36:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BB56F3858C53 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1685702170; bh=6+l+OqrOxjb5bR75E/F/B2Pcjk6AS2GgGvJXnNieQjE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=P9YR8W6rWW8SYY9+TcRexKzcDAgOR74mO6IOl1rB6qbNtS6g4XYMNg+wp+7RDjPHm CvOKAvgYerS18gni/R5uSGfe4ZAzTS+H+kc1O4lgb61bUNwXP1kafiqymF9/oowte7 PRZ3eXWfbZ2v2yGwFxW053oCOBObD7eOOhMfiZHw= From: "uweigand at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug rtl-optimization/101188] [postreload] Uses content of a clobbered register Date: Fri, 02 Jun 2023 10:36:08 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: rtl-optimization X-Bugzilla-Version: 11.1.0 X-Bugzilla-Keywords: ra, wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: uweigand at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: cc Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D101188 Ulrich Weigand changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |uweigand at gcc dot gnu.org --- Comment #12 from Ulrich Weigand --- Sorry for not responding earlier, I've been out on vacation. I think your root cause analysis is correct. In this part of code: if (success) delete_insn (insn); changed |=3D success; insn =3D next; move2add_record_mode (reg); reg_offset[regno] =3D trunc_int_for_mode (added_offset + base_offset, mode); continue; the intent seems to be to manually update the move2add data structures to account for the effects of "next", because the default logic is now skipped= for the "next" insn. That's why in particular the reg mode and offset are manu= ally calculated. This manual logic however is really only correct if "next" is actually just= a simple SET. Reading the comment before the whole loop: /* For simplicity, we only perform this optimization on straightforward SETs. */ makes me suspect the original author assumed that "next" is in fact a straightforward SET here as well. This is however not true due to behavior= of the "single_set" extractor. (I'm wondering if "single_set" used to be defi= ned differently back in the days?) Your fix does look correct to me as far as handling parallel CLOBBERs go.=20 However, looking at "single_set", it seems there is yet another case: the extractor also accepts a parallel of two or more SETs, as long as all except one of those SETs have destinations that are dead. These cases would still= not be handled correctly with your patch, I think. I'm wondering whether it is even worthwhile to attempt to cover those cases= .=20 Maybe a more straightforward fix would be to keep in line with the above-mentioned comment about "straightforward SETs" and just check for a single SET directly instead of using "single_set" here. Do you think this would miss any important optimizations?=