From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id C75C239450EB; Mon, 6 Apr 2020 13:40:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C75C239450EB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1586180459; bh=AD5f9aBUKQ8j2i2dkfeQ6GHBVlAs4MgA+foovzFKC9Q=; h=From:To:Subject:Date:In-Reply-To:References:From; b=dJreM4gCPgta3lzhMjTyPX6v7SznnzJzLO5QbYPw+phxYhXibb+dYW5YY6/7hdwD1 PifH4ZMav5w1zhrPGZe8giecJjL+7QBSk50jo+UEL6wy2vDe8evIbOJTQeG+t67hQa HDuYyPg7pY8IhAff7dzwwjKAYfJ5TPjcl/5zh1zA= From: "jamborm at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/94482] [8/9/10 Regression] Inserting into vector with optimization enabled on x86 generates incorrect result Date: Mon, 06 Apr 2020 13:40:59 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: tree-optimization X-Bugzilla-Version: 9.3.0 X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: jamborm at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: 8.5 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 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Apr 2020 13:40:59 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D94482 --- Comment #21 from Martin Jambor --- As Richi already found out, the path in sra_modify_expr handling type incompatible replacement does not work when the replaced expr comes from within a BIT_FIELD_REF - it does only half of what is necessary. A conservative (not yet much tested) fix would be to emit a full RMW: *** /tmp/UTN9NX_tree-sra.c Mon Apr 6 15:28:23 2020 --- gcc/tree-sra.c Mon Apr 6 15:22:40 2020 *************** sra_modify_expr (tree *expr, gimple_stmt *** 3742,3768 **** ref =3D build_ref_for_model (loc, orig_expr, 0, access, gsi, fals= e); ! if (write) { gassign *stmt; if (access->grp_partial_lhs) ! ref =3D force_gimple_operand_gsi (gsi, ref, true, NULL_TREE, ! false, GSI_NEW_STMT); ! stmt =3D gimple_build_assign (repl, ref); gimple_set_location (stmt, loc); ! gsi_insert_after (gsi, stmt, GSI_NEW_STMT); } ! else { gassign *stmt; if (access->grp_partial_lhs) ! repl =3D force_gimple_operand_gsi (gsi, repl, true, NULL_TR= EE, ! true, GSI_SAME_STMT); ! stmt =3D gimple_build_assign (ref, repl); gimple_set_location (stmt, loc); ! gsi_insert_before (gsi, stmt, GSI_SAME_STMT); } } else --- 3742,3771 ---- ref =3D build_ref_for_model (loc, orig_expr, 0, access, gsi, fals= e); ! if (!write || bfr) { gassign *stmt; + tree src =3D repl; if (access->grp_partial_lhs) ! src =3D force_gimple_operand_gsi (gsi, repl, true, NULL_TRE= E, ! true, GSI_SAME_STMT); ! stmt =3D gimple_build_assign (ref, src); gimple_set_location (stmt, loc); ! gsi_insert_before (gsi, stmt, GSI_SAME_STMT); } ! if (bfr) ! ref =3D unshare_expr (ref); ! if (write || bfr) { gassign *stmt; if (access->grp_partial_lhs) ! ref =3D force_gimple_operand_gsi (gsi, ref, true, NULL_TREE, ! false, GSI_NEW_STMT); ! stmt =3D gimple_build_assign (repl, ref); gimple_set_location (stmt, loc); ! gsi_insert_after (gsi, stmt, GSI_NEW_STMT); } } else But I wonder whether we care about type incompatibility within a B_F_R at all - isn't B_F_R also an implicit V_C_E, always looking at the binary image? So perhaps something as simple as the following might work? diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index b2056b58750..d22b03814d2 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -3736,7 +3736,7 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gs= i, bool write) be accessed as a different type too, potentially creating a need = for type conversion (see PR42196) and when scalarized unions are invo= lved in assembler statements (see PR42398). */ - if (!useless_type_conversion_p (type, access->type)) + if (!bfr && !useless_type_conversion_p (type, access->type)) { tree ref; I'll test both options ...and it seems we need the RMW one to handle REALPART_EXPR and IMAGPART_EXPR.=