public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Alexandre Oliva <aoliva@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/users/aoliva/heads/testme)] -finline-stringops: check base blksize for memset [PR112778] Date: Mon, 11 Dec 2023 16:09:16 +0000 (GMT) [thread overview] Message-ID: <20231211160916.B9B5138582A9@sourceware.org> (raw) https://gcc.gnu.org/g:248f3fa50078aa23e8c630ff5baaa79101c54667 commit 248f3fa50078aa23e8c630ff5baaa79101c54667 Author: Alexandre Oliva <oliva@adacore.com> Date: Fri Dec 8 21:41:47 2023 -0300 -finline-stringops: check base blksize for memset [PR112778] The recently-added logic for -finline-stringops=memset introduced an assumption that doesn't necessarily hold, namely, that can_store_by_pieces of a larger size implies can_store_by_pieces by smaller sizes. Checks for all sizes the by-multiple-pieces machinery might use before committing to an expansion pattern. for gcc/ChangeLog PR target/112778 * builtins.cc (can_store_by_multiple_pieces): New. (try_store_by_multiple_pieces): Call it. for gcc/testsuite/ChangeLog PR target/112778 * gcc.dg/inline-mem-cmp-pr112778.c: New. Diff: --- gcc/builtins.cc | 57 ++++++++++++++++++++++---- gcc/expr.cc | 16 +++----- gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c | 10 +++++ 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 12a535d313f..f6c96498f07 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -4284,6 +4284,40 @@ expand_builtin_memset (tree exp, rtx target, machine_mode mode) return expand_builtin_memset_args (dest, val, len, target, mode, exp); } +/* Check that store_by_pieces allows BITS + LEN (so that we don't + expand something too unreasonably long), and every power of 2 in + BITS. It is assumed that LEN has already been tested by + itself. */ +static bool +can_store_by_multiple_pieces (unsigned HOST_WIDE_INT bits, + by_pieces_constfn constfun, + void *constfundata, unsigned int align, + bool memsetp, + unsigned HOST_WIDE_INT len) +{ + if (bits + && !can_store_by_pieces (bits + len, constfun, constfundata, + align, memsetp)) + return false; + + /* BITS set are expected to be generally in the low range and + contiguous. We do NOT want to repeat the test above in case BITS + has a single bit set, so we terminate the loop when BITS == BIT. + In the unlikely case that BITS has the MSB set, also terminate in + case BIT gets shifted out. */ + for (unsigned HOST_WIDE_INT bit = 1; bit < bits && bit; bit <<= 1) + { + if ((bits & bit) == 0) + continue; + + if (!can_store_by_pieces (bit, constfun, constfundata, + align, memsetp)) + return false; + } + + return true; +} + /* Try to store VAL (or, if NULL_RTX, VALC) in LEN bytes starting at TO. Return TRUE if successful, FALSE otherwise. TO is assumed to be aligned at an ALIGN-bits boundary. LEN must be a multiple of @@ -4341,7 +4375,11 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len, else /* Huh, max_len < min_len? Punt. See pr100843.c. */ return false; - if (min_len >= blksize) + if (min_len >= blksize + /* ??? Maybe try smaller fixed-prefix blksizes before + punting? */ + && can_store_by_pieces (blksize, builtin_memset_read_str, + &valc, align, true)) { min_len -= blksize; min_bits = floor_log2 (min_len); @@ -4367,8 +4405,9 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len, happen because of the way max_bits and blksize are related, but it doesn't hurt to test. */ if (blksize > xlenest - || !can_store_by_pieces (xlenest, builtin_memset_read_str, - &valc, align, true)) + || !can_store_by_multiple_pieces (xlenest - blksize, + builtin_memset_read_str, + &valc, align, true, blksize)) { if (!(flag_inline_stringops & ILSOP_MEMSET)) return false; @@ -4386,17 +4425,17 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len, of overflow. */ if (max_bits < orig_max_bits && xlenest + blksize >= xlenest - && can_store_by_pieces (xlenest + blksize, - builtin_memset_read_str, - &valc, align, true)) + && can_store_by_multiple_pieces (xlenest, + builtin_memset_read_str, + &valc, align, true, blksize)) { max_loop = true; break; } if (blksize - && can_store_by_pieces (xlenest, - builtin_memset_read_str, - &valc, align, true)) + && can_store_by_multiple_pieces (xlenest, + builtin_memset_read_str, + &valc, align, true, 0)) { max_len += blksize; min_len += blksize; diff --git a/gcc/expr.cc b/gcc/expr.cc index 178b3ec6d5a..530dcdd8e3b 100644 --- a/gcc/expr.cc +++ b/gcc/expr.cc @@ -2451,15 +2451,12 @@ emit_block_move_via_loop (rtx x, rtx y, rtx size, opt_scalar_int_mode int_move_mode = int_mode_for_size (incr * BITS_PER_UNIT, 1); - if (!int_move_mode.exists () - || (GET_MODE_BITSIZE (as_a <scalar_int_mode> (int_move_mode)) - != incr * BITS_PER_UNIT)) + if (!int_move_mode.exists (&move_mode) + || (GET_MODE_BITSIZE (mode_mode) != incr * BITS_PER_UNIT)) { move_mode = BLKmode; gcc_checking_assert (can_move_by_pieces (incr, align)); } - else - move_mode = as_a <scalar_int_mode> (int_move_mode); x_addr = force_operand (XEXP (x, 0), NULL_RTX); y_addr = force_operand (XEXP (y, 0), NULL_RTX); @@ -2705,16 +2702,13 @@ emit_block_cmp_via_loop (rtx x, rtx y, rtx len, tree len_type, rtx target, opt_scalar_int_mode int_cmp_mode = int_mode_for_size (incr * BITS_PER_UNIT, 1); - if (!int_cmp_mode.exists () - || (GET_MODE_BITSIZE (as_a <scalar_int_mode> (int_cmp_mode)) - != incr * BITS_PER_UNIT) - || !can_compare_p (NE, as_a <scalar_int_mode> (int_cmp_mode), ccp_jump)) + if (!int_cmp_mode.exists (&cmp_mode) + || (GET_MODE_BITSIZE (cmp_mode) != incr * BITS_PER_UNIT) + || !can_compare_p (NE, cmp_mode, ccp_jump)) { cmp_mode = BLKmode; gcc_checking_assert (incr != 1); } - else - cmp_mode = as_a <scalar_int_mode> (int_cmp_mode); /* Save the base addresses. */ x_addr = force_operand (XEXP (x, 0), NULL_RTX); diff --git a/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c new file mode 100644 index 00000000000..fdfc5b6f28c --- /dev/null +++ b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-finline-stringops" } */ + +char buf[3]; + +int +f () +{ + __builtin_memset (buf, 'v', 3); +}
next reply other threads:[~2023-12-11 16:09 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-12-11 16:09 Alexandre Oliva [this message] -- strict thread matches above, loose matches on Subject: below -- 2023-12-11 16:20 Alexandre Oliva 2023-12-11 16:17 Alexandre Oliva 2023-12-09 5:22 Alexandre Oliva 2023-12-09 5:02 Alexandre Oliva 2023-12-09 2:02 Alexandre Oliva 2023-12-09 1:49 Alexandre Oliva 2023-12-09 1:37 Alexandre Oliva
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231211160916.B9B5138582A9@sourceware.org \ --to=aoliva@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).