On Dec 10, 2024, Richard Biener <richard.guent...@gmail.com> wrote: > On Mon, Dec 9, 2024 at 8:55 PM Alexandre Oliva <ol...@adacore.com> wrote: >> Regstrapped on x86_64-linux-gnu. The aarch64 CI had reported a >> regression with v3, so I'm also running a regstrap on aarch64-linux-gnu, >> but it will still be a while. Ok to install if it passes?
> OK. Thanks. The aarch64-linux-gnu bootstrap revealed a latent surprise, in the form of a bootstrap failure, that this incremental patch addresses. I'll hold off from installing the larger patch without a fix for this. A bootstrap on aarch64-linux-gnu revealed that sometimes (for example, when building shorten_branches in final.cc) we will find such things as MEM <unsigned int>, where unsigned int happen to be a variant of the original unsigned int type, given 64-bit alignment. This unusual alignment circumstance caused get_best_mode to choose DImode instead of SImode, and that failed gimple verification because there aren't that many bits in the unsigned int object. Arrange for alignment to saturate at the inner object size to avoid tripping this error. Regstrapping on x86_64-linux-gnu. Regstrapped an earlier version without lr_align on aarch64-linux-gnu. Ok to install? for gcc/ChangeLog * gimple-fold.cc (fold_truth_andor_for_ifcombine): Saturate align at inner object size. --- gcc/gimple-fold.cc | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc index a31fc283d51b0..967356a950192 100644 --- a/gcc/gimple-fold.cc +++ b/gcc/gimple-fold.cc @@ -8204,24 +8204,31 @@ fold_truth_andor_for_ifcombine (enum tree_code code, tree truth_type, to be relative to a field of that size. */ first_bit = MIN (ll_bitpos, rl_bitpos); end_bit = MAX (ll_bitpos + ll_bitsize, rl_bitpos + rl_bitsize); + HOST_WIDE_INT ll_align = TYPE_ALIGN (TREE_TYPE (ll_inner)); + /* Guard from types with wider-than-size alignment. We must not widen the + load beyond its total size. This is rare. */ + while (ll_align > BITS_PER_UNIT + && TYPE_SIZE (TREE_TYPE (ll_inner)) + && uniform_integer_cst_p (TYPE_SIZE (TREE_TYPE (ll_inner))) + && known_gt ((unsigned HOST_WIDE_INT)ll_align, + tree_to_poly_uint64 (TYPE_SIZE (TREE_TYPE (ll_inner))))) + ll_align /= 2; if (get_best_mode (end_bit - first_bit, first_bit, 0, 0, - TYPE_ALIGN (TREE_TYPE (ll_inner)), BITS_PER_WORD, - volatilep, &lnmode)) + ll_align, BITS_PER_WORD, volatilep, &lnmode)) l_split_load = false; else { /* Consider the possibility of recombining loads if any of the fields straddles across an alignment boundary, so that either part can be loaded along with the other field. */ - HOST_WIDE_INT align = TYPE_ALIGN (TREE_TYPE (ll_inner)); HOST_WIDE_INT boundary = compute_split_boundary_from_align - (align, ll_bitpos, ll_bitsize, rl_bitpos, rl_bitsize); + (ll_align, ll_bitpos, ll_bitsize, rl_bitpos, rl_bitsize); if (boundary < 0 || !get_best_mode (boundary - first_bit, first_bit, 0, 0, - align, BITS_PER_WORD, volatilep, &lnmode) + ll_align, BITS_PER_WORD, volatilep, &lnmode) || !get_best_mode (end_bit - boundary, boundary, 0, 0, - align, BITS_PER_WORD, volatilep, &lnmode2)) + ll_align, BITS_PER_WORD, volatilep, &lnmode2)) return 0; /* If we can't have a single load, but can with two, figure out whether @@ -8368,16 +8375,24 @@ fold_truth_andor_for_ifcombine (enum tree_code code, tree truth_type, and then we use two separate compares. */ first_bit = MIN (lr_bitpos, rr_bitpos); end_bit = MAX (lr_bitpos + lr_bitsize, rr_bitpos + rr_bitsize); + HOST_WIDE_INT lr_align = TYPE_ALIGN (TREE_TYPE (lr_inner)); + /* Guard from types with wider-than-size alignment. We must not widen the + load beyond its total size. This is rare. */ + while (lr_align > BITS_PER_UNIT + && TYPE_SIZE (TREE_TYPE (lr_inner)) + && uniform_integer_cst_p (TYPE_SIZE (TREE_TYPE (lr_inner))) + && known_gt ((unsigned HOST_WIDE_INT)lr_align, + tree_to_poly_uint64 (TYPE_SIZE + (TREE_TYPE (lr_inner))))) + lr_align /= 2; if (!get_best_mode (end_bit - first_bit, first_bit, 0, 0, - TYPE_ALIGN (TREE_TYPE (lr_inner)), BITS_PER_WORD, - volatilep, &rnmode)) + lr_align, BITS_PER_WORD, volatilep, &rnmode)) { /* Consider the possibility of recombining loads if any of the fields straddles across an alignment boundary, so that either part can be loaded along with the other field. */ - HOST_WIDE_INT align = TYPE_ALIGN (TREE_TYPE (lr_inner)); HOST_WIDE_INT boundary = compute_split_boundary_from_align - (align, lr_bitpos, lr_bitsize, rr_bitpos, rr_bitsize); + (lr_align, lr_bitpos, lr_bitsize, rr_bitpos, rr_bitsize); if (boundary < 0 /* If we're to split both, make sure the split point is @@ -8386,9 +8401,9 @@ fold_truth_andor_for_ifcombine (enum tree_code code, tree truth_type, && (boundary - lr_bitpos != (lnbitpos + GET_MODE_BITSIZE (lnmode)) - ll_bitpos)) || !get_best_mode (boundary - first_bit, first_bit, 0, 0, - align, BITS_PER_WORD, volatilep, &rnmode) + lr_align, BITS_PER_WORD, volatilep, &rnmode) || !get_best_mode (end_bit - boundary, boundary, 0, 0, - align, BITS_PER_WORD, volatilep, &rnmode2)) + lr_align, BITS_PER_WORD, volatilep, &rnmode2)) return 0; r_split_load = true; -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive