From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 1B64D39DDD9B; Thu, 8 Dec 2022 11:07:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1B64D39DDD9B 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-out2.suse.de (Postfix) with ESMTP id 17E94208B1; Thu, 8 Dec 2022 11:07:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1670497629; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=zd9T5ZIaKizuzKKQ5x9r6H4bINJ15bMpm58X1DX5nR4=; b=IQqMSDesLovyZM9kBbCbsYH8sH2g+3by58sGzbDMSAaC7TtS03dEG4wHX3Qx3J/3z+qWHa RW1n0gzXSJVhZzCFLh6iQRsUvklkb8wEQ520EJ8aHnNoOV43oMI6IWvbDNIuaWtm3EOuGb Nm+mX7IXBsJ3F6dxR0r80Z5lBUdlcCc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1670497629; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=zd9T5ZIaKizuzKKQ5x9r6H4bINJ15bMpm58X1DX5nR4=; b=K0/E4buUcGTbNJmOhJfwBOIxxSHiDhjDllWKd1t0F3V1gh2lQt8puGfbCdfDoRvZiwOJjv orWZAJyLjNKK7sAA== 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 D8D702C142; Thu, 8 Dec 2022 11:07:06 +0000 (UTC) Date: Thu, 8 Dec 2022 11:07:08 +0000 (UTC) From: Richard Biener To: gcc-patches@gcc.gnu.org cc: fortran@gcc.gnu.org Subject: [PATCH] tree-optimization/99919 - bogus uninit diagnostic with bitfield guards Message-ID: 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=-9.7 required=5.0 tests=BAYES_00,DKIM_INVALID,DKIM_SIGNED,GIT_PATCH_0,KAM_DMARC_STATUS,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: For the testcase in this PR what fold-const.cc optimize_bit_field_compare does to bitfield against constant compares is confusing the uninit predicate analysis and it also makes SRA obfuscate instead of optimize the code. We've long had the opinion that those optimizations are premature but we do not have any replacement for the more complicated ones combining multiple bitfield tests. The following disables mangling the case of a single bitfield test against constants but preserving the existing diagnostic and optimization to a compile-time determined value. This requires silencing a bogus uninit diagnostic in the Fortran frontend which I've done in a minimal way, avoiding initializing the 40 byte symbol_attribute structure. There's several issues, one is the flag_coarrays is a global variable likely not CSEd to help the uninit predicate analysis, the other is us short-circuiting the flag_coarray == GFC_FCOARRAY_LIB && lhs_caf_attr.codimension accesses as both have no side-effects so the guard isn't effective. If the frontend folks are happy with this I can localize both lhs_caf_attr and rhs_caf_attr and copy out the two only flags tested by the code instead of the somewhat incomplete approach in the patch. Any opinions here? Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. OK for the fortran parts? Thanks, Richard. PR tree-optimization/99919 * fold-const.cc (optimize_bit_field_compare): Disable transforming the bitfield against constant compare optimization if the result is not statically determinable. gcc/fortran/ * trans-expr.cc (gfc_trans_assignment_1): Split out lhs_codimension from lhs_caf_attr to avoid bogus uninit diagnostics. * gcc.dg/uninit-pr99919.c: New testcase. --- gcc/fold-const.cc | 37 +++------------------------ gcc/fortran/trans-expr.cc | 6 +++-- gcc/testsuite/gcc.dg/uninit-pr99919.c | 22 ++++++++++++++++ 3 files changed, 30 insertions(+), 35 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/uninit-pr99919.c diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index cdfe3f50ae3..b72cc0a1d51 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -4559,7 +4559,6 @@ optimize_bit_field_compare (location_t loc, enum tree_code code, { poly_int64 plbitpos, plbitsize, rbitpos, rbitsize; HOST_WIDE_INT lbitpos, lbitsize, nbitpos, nbitsize; - tree type = TREE_TYPE (lhs); tree unsigned_type; int const_p = TREE_CODE (rhs) == INTEGER_CST; machine_mode lmode, rmode; @@ -4667,13 +4666,7 @@ optimize_bit_field_compare (location_t loc, enum tree_code code, } /* Otherwise, we are handling the constant case. See if the constant is too - big for the field. Warn and return a tree for 0 (false) if so. We do - this not only for its own sake, but to avoid having to test for this - error case below. If we didn't, we might generate wrong code. - - For unsigned fields, the constant shifted right by the field length should - be all zero. For signed fields, the high-order bits should agree with - the sign bit. */ + big for the field. Warn and return a tree for 0 (false) if so. */ if (lunsignedp) { @@ -4695,31 +4688,9 @@ optimize_bit_field_compare (location_t loc, enum tree_code code, } } - if (nbitpos < 0) - return 0; - - /* Single-bit compares should always be against zero. */ - if (lbitsize == 1 && ! integer_zerop (rhs)) - { - code = code == EQ_EXPR ? NE_EXPR : EQ_EXPR; - rhs = build_int_cst (type, 0); - } - - /* Make a new bitfield reference, shift the constant over the - appropriate number of bits and mask it with the computed mask - (in case this was a signed field). If we changed it, make a new one. */ - lhs = make_bit_field_ref (loc, linner, lhs, unsigned_type, - nbitsize, nbitpos, 1, lreversep); - - rhs = const_binop (BIT_AND_EXPR, - const_binop (LSHIFT_EXPR, - fold_convert_loc (loc, unsigned_type, rhs), - size_int (lbitpos)), - mask); - - lhs = build2_loc (loc, code, compare_type, - build2 (BIT_AND_EXPR, unsigned_type, lhs, mask), rhs); - return lhs; + /* Otherwise do not prematurely optimize compares of bitfield members + to constants. */ + return 0; } /* Subroutine for fold_truth_andor_1: decode a field reference. diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index b95c5cf2f96..12c7dd7f26a 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -11654,9 +11654,11 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, /* Only analyze the expressions for coarray properties, when in coarray-lib mode. */ + bool lhs_codimension = false; if (flag_coarray == GFC_FCOARRAY_LIB) { lhs_caf_attr = gfc_caf_attr (expr1, false, &lhs_refs_comp); + lhs_codimension = lhs_caf_attr.codimension; rhs_caf_attr = gfc_caf_attr (expr2, false, &rhs_refs_comp); } @@ -11738,7 +11740,7 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, /* Translate the expression. */ rse.want_coarray = flag_coarray == GFC_FCOARRAY_LIB && init_flag - && lhs_caf_attr.codimension; + && lhs_codimension; gfc_conv_expr (&rse, expr2); /* Deal with the case of a scalar class function assigned to a derived type. */ @@ -11913,7 +11915,7 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, } } else if (flag_coarray == GFC_FCOARRAY_LIB - && lhs_caf_attr.codimension && rhs_caf_attr.codimension + && lhs_codimension && rhs_caf_attr.codimension && ((lhs_caf_attr.allocatable && lhs_refs_comp) || (rhs_caf_attr.allocatable && rhs_refs_comp))) { diff --git a/gcc/testsuite/gcc.dg/uninit-pr99919.c b/gcc/testsuite/gcc.dg/uninit-pr99919.c new file mode 100644 index 00000000000..5ee0e6727b3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/uninit-pr99919.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wuninitialized" } */ + +struct B { _Bool i: 1; _Bool j: 1; }; + +_Bool z; + +void g (struct B b) +{ + _Bool x; + + if (b.i) + b.j = 0; + else + { + b.j = b.i; + x = b.i; + } + + if (b.j) + z = x; /* { dg-bogus "uninitialized" } */ +} -- 2.35.3