Alexandre Oliva <ol...@adacore.com> writes:
> On Feb 6, 2025, Sam James <s...@gentoo.org> wrote:
>
>> Richard Biener <richard.guent...@gmail.com> writes:
>>> On Thu, Feb 6, 2025 at 2:41 PM Alexandre Oliva <ol...@adacore.com> wrote:
>>>>
>>>> On Jan 27, 2025, Richard Biener <richard.guent...@gmail.com> wrote:
>>>> > (I see the assert is no longer in the patch).
>>>>
>>>> That's because the assert went in as part of an earlier patch. I take
>>>> it it should be backed out along with the to-be-split-out bits above,
>>>> right?
>>>
>>> Yes.
>>>
>>> (IIRC there's also a PR tripping over this or a similar assert)
>
>> Right, PR118706.
>
> Thanks. I've added its testcase to the patch below, reverted the
> assert, and dropped the other unwanted bits. Regstrapped on
> x86_64-linux-gnu. Ok to install?
Thanks. BTW, there's another for you at PR118805 (sorry).
>
>
>
> If decode_field_reference finds a load that accesses past the inner
> object's size, bail out.
>
> Drop the too-strict assert.
>
>
> for gcc/ChangeLog
>
> PR tree-optimization/118514
> PR tree-optimization/118706
> * gimple-fold.cc (decode_field_reference): Refuse to consider
> merging out-of-bounds BIT_FIELD_REFs.
> (make_bit_field_load): Drop too-strict assert.
> * tree-eh.cc (bit_field_ref_in_bounds_p): Rename to...
> (access_in_bounds_of_type_p): ... this. Change interface,
> export.
> (tree_could_trap_p): Adjust.
> * tree-eh.h (access_in_bounds_of_type_p): Declare.
>
> for gcc/testsuite/ChangeLog
>
> PR tree-optimization/118514
> PR tree-optimization/118706
> * gcc.dg/field-merge-25.c: New.
> ---
> gcc/gimple-fold.cc | 11 ++---------
> gcc/testsuite/gcc.dg/field-merge-25.c | 15 +++++++++++++++
> gcc/tree-eh.cc | 25 +++++++++++++------------
> gcc/tree-eh.h | 1 +
> 4 files changed, 31 insertions(+), 21 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/field-merge-25.c
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 45485782cdf91..29191685a43c5 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -7686,10 +7686,8 @@ decode_field_reference (tree *pexp, HOST_WIDE_INT
> *pbitsize,
> || bs <= shiftrt
> || offset != 0
> || TREE_CODE (inner) == PLACEHOLDER_EXPR
> - /* Reject out-of-bound accesses (PR79731). */
> - || (! AGGREGATE_TYPE_P (TREE_TYPE (inner))
> - && compare_tree_int (TYPE_SIZE (TREE_TYPE (inner)),
> - bp + bs) < 0)
> + /* Reject out-of-bound accesses (PR79731, PR118514). */
> + || !access_in_bounds_of_type_p (TREE_TYPE (inner), bs, bp)
> || (INTEGRAL_TYPE_P (TREE_TYPE (inner))
> && !type_has_mode_precision_p (TREE_TYPE (inner))))
> return NULL_TREE;
> @@ -7859,11 +7857,6 @@ make_bit_field_load (location_t loc, tree inner, tree
> orig_inner, tree type,
> gimple *new_stmt = gsi_stmt (i);
> if (gimple_has_mem_ops (new_stmt))
> gimple_set_vuse (new_stmt, reaching_vuse);
> - gcc_checking_assert (! (gimple_assign_load_p (point)
> - && gimple_assign_load_p (new_stmt))
> - || (tree_could_trap_p (gimple_assign_rhs1 (point))
> - == tree_could_trap_p (gimple_assign_rhs1
> - (new_stmt))));
> }
>
> gimple_stmt_iterator gsi = gsi_for_stmt (point);
> diff --git a/gcc/testsuite/gcc.dg/field-merge-25.c
> b/gcc/testsuite/gcc.dg/field-merge-25.c
> new file mode 100644
> index 0000000000000..e769b0ae7b846
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/field-merge-25.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fno-tree-fre" } */
> +
> +/* PR tree-optimization/118706 */
> +
> +int a[1][1][3], b;
> +int main() {
> + int c = -1;
> + while (b) {
> + if (a[c][c][6])
> + break;
> + if (a[0][0][0])
> + break;
> + }
> +}
> diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
> index 7015189a2de83..a4d59954c0597 100644
> --- a/gcc/tree-eh.cc
> +++ b/gcc/tree-eh.cc
> @@ -2646,24 +2646,22 @@ range_in_array_bounds_p (tree ref)
> return true;
> }
>
> -/* Return true iff EXPR, a BIT_FIELD_REF, accesses a bit range that is known
> to
> - be in bounds for the referred operand type. */
> +/* Return true iff a BIT_FIELD_REF <(TYPE)???, SIZE, OFFSET> would access a
> bit
> + range that is known to be in bounds for TYPE. */
>
> -static bool
> -bit_field_ref_in_bounds_p (tree expr)
> +bool
> +access_in_bounds_of_type_p (tree type, poly_uint64 size, poly_uint64 offset)
> {
> - tree size_tree;
> - poly_uint64 size_max, min, wid, max;
> + tree type_size_tree;
> + poly_uint64 type_size_max, min = offset, wid = size, max;
>
> - size_tree = TYPE_SIZE (TREE_TYPE (TREE_OPERAND (expr, 0)));
> - if (!size_tree || !poly_int_tree_p (size_tree, &size_max))
> + type_size_tree = TYPE_SIZE (type);
> + if (!type_size_tree || !poly_int_tree_p (type_size_tree, &type_size_max))
> return false;
>
> - min = bit_field_offset (expr);
> - wid = bit_field_size (expr);
> max = min + wid;
> if (maybe_lt (max, min)
> - || maybe_lt (size_max, max))
> + || maybe_lt (type_size_max, max))
> return false;
>
> return true;
> @@ -2712,7 +2710,10 @@ tree_could_trap_p (tree expr)
> switch (code)
> {
> case BIT_FIELD_REF:
> - if (DECL_P (TREE_OPERAND (expr, 0)) && !bit_field_ref_in_bounds_p
> (expr))
> + if (DECL_P (TREE_OPERAND (expr, 0))
> + && !access_in_bounds_of_type_p (TREE_TYPE (TREE_OPERAND (expr, 0)),
> + bit_field_size (expr),
> + bit_field_offset (expr)))
> return true;
> /* Fall through. */
>
> diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h
> index 69fe193f1b82f..1fe6de80c42e7 100644
> --- a/gcc/tree-eh.h
> +++ b/gcc/tree-eh.h
> @@ -36,6 +36,7 @@ extern void redirect_eh_dispatch_edge (geh_dispatch *,
> edge, basic_block);
> extern bool operation_could_trap_helper_p (enum tree_code, bool, bool, bool,
> bool, tree, bool *);
> extern bool operation_could_trap_p (enum tree_code, bool, bool, tree);
> +extern bool access_in_bounds_of_type_p (tree, poly_uint64, poly_uint64);
> extern bool tree_could_trap_p (tree);
> extern tree rewrite_to_non_trapping_overflow (tree);
> extern bool stmt_could_throw_p (function *, gimple *);
