From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 302243858004; Tue, 16 Nov 2021 18:18:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 302243858004 From: "amacleod at redhat dot com" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad Date: Tue, 16 Nov 2021 18:18:22 +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: 11.1.0 X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: amacleod at redhat dot com 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: 11.3 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: Tue, 16 Nov 2021 18:18:22 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D103255 --- Comment #6 from Andrew Macleod --- (In reply to Jakub Jelinek from comment #5) > I think the bug is in: > else if (flag_delete_null_pointer_checks > && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))) > { > /* For -fdelete-null-pointer-checks -fno-wrapv-pointer we don't > allow going from non-NULL pointer to NULL. */ > if(!range_includes_zero_p (&r)) > return true; > } >=20 > If !off_cst (not this case), I think before return true here we need > r =3D range_nonzero (TREE_TYPE (gimple_assign_rhs1 (stmt))); > - we are going from the base pointer which is known not to be NULL to tha= t + > some unknown offset, all we can say is that the result is not NULL, but we > don't really know the exact details. > For the off_cst case (probably only when it is actually constant, i.e. > off_cst && off.to_constant (&offi), we could use something based on r, b= ut > will need to add the offi / BITS_PER_UNIT addend to the range, probably > verify the result doesn't include zero and if it would, use that > range_nonzero too. >=20 > For the addition, not sure if that would be range_op_handler (PLUS_EXPR, > type)->fold_range or range_op_handler (POINTER_PLUS_EXPR, type)->fold_ran= ge > or what. >=20 > Because, with the code as is, we end up with the original r for the base > hdr_3: struct header * [4194336B, 4194336B] > and instead of returning range unsigned int * [4194340B, 4194340B] we ret= urn > incorrect unsigned int * [4194336B, 4194336B] >=20 > Or does this code rely on pointer ranges to be always just range_zero, > range_nonzero or varying and nothing else? If so, it should normalize > INTEGER_CSTs used in addresses etc. into range_zero or range_nonzero and > nothing else... That code is an import from vr-values, and in gcc 10 it did just what we do: if ((off_cst && known_eq (off, 0)) || (flag_delete_null_pointer_checks && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr)))) { const value_range_equiv *vr =3D get_value_range (TREE_OPERAND (base, 0)); if (!range_includes_zero_p (vr)) return true; } Perhaps it later sanitizes it to non-zero... Ah, indeed: else if (vrp_stmt_computes_nonzero (stmt)) { vr->set_nonzero (type); vr->equiv_clear (); } nothing is suppose to depend on the zero/nonzero directly. Pointer tracking= is a little schizophrenic. Sometimes it just tracks zero/nonzero, other times= it uses integral calculations to come up with ranges.. especially if its gone = back or forth to an integral value via casting.=20 If we know its a constant, and we know the offset is a constant, then there would be no harm in doing the math and coming up with the right value... j= ust as you suggest, with a check to make sure if zero has reappeared that we re= move it.=20=20=20 Or we could restore previous behaviour if we simply changed it to non-zero = as well before returning true. Either would be correct/fine.=