On Dec 20, 2024, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Dec 18, 2024 at 12:59:11AM -0300, Alexandre Oliva wrote:
>> * gcc.dg/field-merge-16.c: New.
> Note the test FAILs on i686-linux or on x86_64-linux with -m32.
Indeed, thanks. Here's a fix.
On 32-bit hosts, data types with 64-bit alignment aren't getting
treated as desired by ifcombine field-merging: we limit the choice of
modes at BITS_PER_WORD sizes, but when deciding the boundary for a
split, we'd limit the choice only by the alignment, so we wouldn't
even consider a split at an odd 32-bit boundary. Fix that by limiting
the boundary choice by word choice as well.
Now, this would still leave misaligned 64-bit fields in 64-bit-aligned
data structures unhandled by ifcombine on 32-bit hosts. We already
need to loading them as double words, and if they're not byte-aligned,
the code gets really ugly, but ifcombine could improve it if it allows
double-word loads as a last resort. I've added that.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/ChangeLog
* gimple-fold.cc (fold_truth_andor_for_ifcombine): Limit
boundary choice by word size as well. Try aligned double-word
loads as a last resort.
for gcc/testsuite/ChangeLog
* gcc.dg/field-merge-17.c: New.
---
gcc/gimple-fold.cc | 30 +++++++++++++++++++---
gcc/testsuite/gcc.dg/field-merge-17.c | 46 +++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/field-merge-17.c
diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 2d6e2074416f5..0e832158a47b3 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -8381,16 +8381,40 @@ fold_truth_andor_for_ifcombine (enum tree_code code,
tree truth_type,
{
/* 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. */
+ part can be loaded along with the other field. Since we
+ limit access modes to BITS_PER_WORD, don't exceed that,
+ otherwise on a 32-bit host and a 64-bit-aligned data
+ structure, we'll fail the above for a field that straddles
+ across two words, and would fail here for not even trying to
+ split it at between 32-bit words. */
HOST_WIDE_INT boundary = compute_split_boundary_from_align
- (ll_align, ll_bitpos, ll_bitsize, rl_bitpos, rl_bitsize);
+ (MIN (ll_align, BITS_PER_WORD),
+ ll_bitpos, ll_bitsize, rl_bitpos, rl_bitsize);
if (boundary < 0
|| !get_best_mode (boundary - first_bit, first_bit, 0, ll_end_region,
ll_align, BITS_PER_WORD, volatilep, &lnmode)
|| !get_best_mode (end_bit - boundary, boundary, 0, ll_end_region,
ll_align, BITS_PER_WORD, volatilep, &lnmode2))
- return 0;
+ {
+ if (ll_align <= BITS_PER_WORD)
+ return 0;
+
+ /* As a last resort, try double-word access modes. This
+ enables us to deal with misaligned double-word fields
+ that straddle across 3 separate words. */
+ boundary = compute_split_boundary_from_align
+ (MIN (ll_align, 2 * BITS_PER_WORD),
+ ll_bitpos, ll_bitsize, rl_bitpos, rl_bitsize);
+ if (boundary < 0
+ || !get_best_mode (boundary - first_bit, first_bit,
+ 0, ll_end_region, ll_align, 2 * BITS_PER_WORD,
+ volatilep, &lnmode)
+ || !get_best_mode (end_bit - boundary, boundary,
+ 0, ll_end_region, ll_align, 2 * BITS_PER_WORD,
+ volatilep, &lnmode2))
+ return 0;
+ }
/* If we can't have a single load, but can with two, figure out whether
the two compares can be separated, i.e., whether the entirety of the
diff --git a/gcc/testsuite/gcc.dg/field-merge-17.c
b/gcc/testsuite/gcc.dg/field-merge-17.c
new file mode 100644
index 0000000000000..06c8ec16e86c6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/field-merge-17.c
@@ -0,0 +1,46 @@
+/* { dg-do run } */
+/* { dg-options "-O -fdump-tree-ifcombine-details" } */
+
+/* Check that we can optimize misaligned double-words. */
+
+struct s {
+ short a;
+ long long b;
+ int c;
+ long long d;
+ short e;
+} __attribute__ ((packed, aligned (8)));
+
+struct s p = { 0, 0, 0, 0, 0 };
+
+__attribute__ ((__noinline__, __noipa__, __noclone__))
+int fp ()
+{
+ if (p.a
+ || p.b
+ || p.c
+ || p.d
+ || p.e)
+ return 1;
+ else
+ return -1;
+}
+
+int main () {
+ /* Unlikely, but play safe. */
+ if (sizeof (long long) == sizeof (short))
+ return 0;
+ if (fp () > 0)
+ __builtin_abort ();
+ unsigned char *pc = (unsigned char *)&p;
+ for (int i = 0; i < sizeof (p); i++)
+ {
+ pc[i] = 1;
+ if (fp () < 0)
+ __builtin_abort ();
+ pc[i] = 0;
+ }
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "optimizing" 4 "ifcombine" } } */
--
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
