From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id B62EC3858D33; Tue, 20 Jun 2023 14:47:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B62EC3858D33 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1687272428; bh=EfkqFZgKbaK1rnHvWD+J/7KsgjZlZlVgkqV5/TmsWLE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=gI36a5yT70+LNqBNcGSRXJE2TSJaHNg1h/DjhXrsz7ELEeaIe5/QfcHBf4zk6kRLB kHvfwrLPnXVJJJNtrdPZbvlR0yS+NXpeEdysPoaTzmC98SbCSnUGgVY7l6bsfbsGnL ZfIur0JeW5fKBggDInQhDJKvtI/bA7oomF0R68cE= From: "manolis.tsamis at vrull dot eu" To: gcc-bugs@gcc.gnu.org Subject: [Bug debug/110308] [14 Regression] ICE on audiofile-0.3.6: RTL: vartrack: Segmentation fault in mode_to_precision(machine_mode) Date: Tue, 20 Jun 2023 14:47:08 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: debug X-Bugzilla-Version: 14.0 X-Bugzilla-Keywords: ice-on-valid-code X-Bugzilla-Severity: normal X-Bugzilla-Who: manolis.tsamis at vrull dot eu X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P1 X-Bugzilla-Assigned-To: ptomsich at gcc dot gnu.org X-Bugzilla-Target-Milestone: 14.0 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: 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=3D110308 --- Comment #7 from manolis.tsamis at vrull dot eu --- Some context for the commit: This change is originally part of an late rtl pass to optimize memory acces= ses. There are a lot of cases (especially involving local arrays) that generate reduntant stack pointer adds for address calculation which can get reduced = to `mov reg, sp`, but without actually propagating these we don't gain somethi= ng. In general it should be good to allow propagation of the stack pointer if t= hat is correct. Now for the actual issue, it looks like my change for that was a bit carele= es and I didn't properly understand the context of cprop-hardreg. > For debug info, propagation of the sp with the extra debug info related s= tuff (ORIGINAL_REGNO and REG_ATTRS) is I think very harmful, but I admit I = haven't tried to understand why that change has been done. Yes, the attachment of ORIGINAL_REGNO and REG_ATTRS to the stack pointer has been accidental and is of course wrong. I have a proposed change below that fixes the segfault by not setting ORIGINAL_REGNO/REG_ATTRS for the stack pointer. My understanding is that th= is should be fine, but I'm still testing that. > So I think there are 2 bugs here. First the lost of debugging info becaus= e of ch, and the latent segfault. I'm still looking into the difference in the debug statements. --cut here-- --- a/gcc/regcprop.cc +++ b/gcc/regcprop.cc @@ -423,7 +423,7 @@ maybe_mode_change (machine_mode orig_mode, machine_mode copy_mode, It's unclear if we need to do the same for other special registers. = */ if (regno =3D=3D STACK_POINTER_REGNUM) { - if (orig_mode =3D=3D new_mode) + if (orig_mode =3D=3D new_mode && new_mode =3D=3D GET_MODE (stack_poi= nter_rtx)) return stack_pointer_rtx; else return NULL_RTX; @@ -487,9 +487,14 @@ find_oldest_value_reg (enum reg_class cl, rtx reg, str= uct value_data *vd) new_rtx =3D maybe_mode_change (oldmode, vd->e[regno].mode, mode, i, regno); if (new_rtx) { - ORIGINAL_REGNO (new_rtx) =3D ORIGINAL_REGNO (reg); - REG_ATTRS (new_rtx) =3D REG_ATTRS (reg); - REG_POINTER (new_rtx) =3D REG_POINTER (reg); + if (new_rtx !=3D stack_pointer_rtx) + { + ORIGINAL_REGNO (new_rtx) =3D ORIGINAL_REGNO (reg); + REG_ATTRS (new_rtx) =3D REG_ATTRS (reg); + REG_POINTER (new_rtx) =3D REG_POINTER (reg); + } + else if (REG_POINTER (new_rtx) !=3D REG_POINTER (reg)) + return NULL_RTX; return new_rtx; } } @@ -965,15 +970,27 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) if (validate_change (insn, &SET_SRC (set), new_rtx, 0)) { - ORIGINAL_REGNO (new_rtx) =3D ORIGINAL_REGNO (src); - REG_ATTRS (new_rtx) =3D REG_ATTRS (src); - REG_POINTER (new_rtx) =3D REG_POINTER (src); - if (dump_file) - fprintf (dump_file, - "insn %u: replaced reg %u with %u\n", - INSN_UID (insn), regno, REGNO (new_rtx)); - changed =3D true; - goto did_replacement; + bool can_change; + if (new_rtx !=3D stack_pointer_rtx) + { + ORIGINAL_REGNO (new_rtx) =3D ORIGINAL_REGNO (src); + REG_ATTRS (new_rtx) =3D REG_ATTRS (src); + REG_POINTER (new_rtx) =3D REG_POINTER (src); + can_change =3D true; + } + else + can_change + =3D (REG_POINTER (new_rtx) =3D=3D REG_POINTER (sr= c)); + + if (can_change) + { + if (dump_file) + fprintf (dump_file, + "insn %u: replaced reg %u with %u\n", + INSN_UID (insn), regno, REGNO (new_rtx= )); + changed =3D true; + goto did_replacement; + } } /* We need to re-extract as validate_change clobbers recog_data. */(In reply to Jakub Jelinek from comment= #5) --cut here--=