public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ifcvt: Support bitfield lowering of multiple-exit loops
@ 2022-11-03 13:43 Andre Vieira (lists)
  2022-11-07 11:05 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Andre Vieira (lists) @ 2022-11-03 13:43 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Biener, Richard Sandiford

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

Hi,

With Tamar's patch 
(https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604880.html) 
enabling the vectorization of early-breaks, I'd like to allow bitfield 
lowering in such loops, which requires the relaxation of allowing 
multiple exits when doing so.  In order to avoid a similar issue to 
PR107275, I hoisted the code that rejects loops with certain types of 
gimple_stmts from 'if_convertible_loop_p_1' to 
'get_loop_body_in_if_conv_order', to avoid trying to lower bitfields in 
loops we are not going to vectorize anyway.  This also ensures 
'ifcvt_local_dce' doesn't accidentally remove statements it shouldn't as 
it will never come across them.  I made sure to add a comment to make 
clear that there is a direct connection between the two and if we were 
to enable vectorization of any other gimple statement we should make 
sure both handle it.

Bootstrapped and regression tested on aarch64-none-linux-gnu and 
x86_64-pc-linux-gnu

gcc/ChangeLog:

         * tree-if-conv.cc (if_convertible_loop_p_1): Move statement 
check loop from here ...
         (get_loop_body_if_conv_order): ... to here.
         (if_convertible_loop_p): Remove single_exit check.
         (tree_if_conversion): Move single_exit check to if-conversion part.

gcc/testsuite/ChangeLog:

         * gcc.dg/vect/vect-bitfield-read-1-not.c: New test.
         * gcc.dg/vect/vect-bitfield-read-2-not.c: New test.
         * gcc.dg/vect/vect-bitfield-read-8.c: New test.
         * gcc.dg/vect/vect-bitfield-read-9.c: New test.

[-- Attachment #2: lower_bitfield_multiple_exits.patch --]
[-- Type: text/plain, Size: 9570 bytes --]

diff --git a/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-1-not.c b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-1-not.c
new file mode 100644
index 0000000000000000000000000000000000000000..0d91067ebb27b1db2b2352975c43bce8b4171e3f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-1-not.c
@@ -0,0 +1,60 @@
+/* { dg-require-effective-target vect_shift } */
+/* { dg-require-effective-target vect_long_long } */
+/* { dg-additional-options { "-fdump-tree-ifcvt-all" } } */
+
+#include <stdarg.h>
+#include "tree-vect.h"
+
+extern void abort(void);
+
+struct s {
+    char a : 4;
+};
+
+#define N 32
+#define ELT0 {0}
+#define ELT1 {1}
+#define ELT2 {2}
+#define ELT3 {3}
+#define RES 56
+struct s A[N]
+  = { ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3};
+
+int __attribute__ ((noipa))
+f(struct s *ptr, unsigned n) {
+    int res = 0;
+    for (int i = 0; i < n; ++i)
+      {
+	switch (ptr[i].a)
+	  {
+	  case 0:
+	    res += ptr[i].a + 1;
+	    break;
+	  case 1:
+	  case 2:
+	  case 3:
+	    res += ptr[i].a;
+	    break;
+	  default:
+	    return 0;
+	  }
+      }
+    return res;
+}
+
+int main (void)
+{
+  check_vect ();
+
+  if (f(&A[0], N) != RES)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "Bitfield OK to lower." "ifcvt" } } */
+
+
diff --git a/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-2-not.c b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-2-not.c
new file mode 100644
index 0000000000000000000000000000000000000000..4ac7b3fc0dfd1c9d0b5e94a2ba6a745545577ec1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-2-not.c
@@ -0,0 +1,49 @@
+/* { dg-require-effective-target vect_shift } */
+/* { dg-require-effective-target vect_long_long } */
+/* { dg-additional-options { "-fdump-tree-ifcvt-all" } } */
+
+#include <stdarg.h>
+#include "tree-vect.h"
+
+extern void abort(void);
+
+struct s {
+    char a : 4;
+};
+
+#define N 32
+#define ELT0 {0}
+#define ELT1 {1}
+#define ELT2 {2}
+#define ELT3 {3}
+#define RES 48
+struct s A[N]
+  = { ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3};
+
+int __attribute__ ((noipa))
+f(struct s *ptr, unsigned n) {
+    int res = 0;
+    for (int i = 0; i < n; ++i)
+      {
+	asm volatile ("" ::: "memory");
+	res += ptr[i].a;
+      }
+    return res;
+}
+
+int main (void)
+{
+  check_vect ();
+
+  if (f(&A[0], N) != RES)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "Bitfield OK to lower." "ifcvt" } } */
+
+
diff --git a/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-8.c b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-8.c
new file mode 100644
index 0000000000000000000000000000000000000000..52cfd33d937ae90f3fe9556716c90e098b768ac8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-8.c
@@ -0,0 +1,49 @@
+/* { dg-require-effective-target vect_int } */
+/* { dg-require-effective-target vect_shift } */
+/* { dg-additional-options { "-fdump-tree-ifcvt-all" } } */
+
+#include <stdarg.h>
+#include "tree-vect.h"
+
+extern void abort(void);
+
+struct s { int i : 31; };
+
+#define ELT0 {0}
+#define ELT1 {1}
+#define ELT2 {2}
+#define ELT3 {3}
+#define ELT4 {4}
+#define N 32
+#define RES 25
+struct s A[N]
+  = { ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT4, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3};
+
+int __attribute__ ((noipa))
+f(struct s *ptr, unsigned n) {
+    int res = 0;
+    for (int i = 0; i < n; ++i)
+      {
+	if (ptr[i].i == 4)
+	  return res;
+	res += ptr[i].i;
+      }
+
+    return res;
+}
+
+int main (void)
+{
+  check_vect ();
+
+  if (f(&A[0], N) != RES)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "Bitfield OK to lower." "ifcvt" } } */
+
diff --git a/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-9.c b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-9.c
new file mode 100644
index 0000000000000000000000000000000000000000..ab814698131a5905def181eeed85d8a3c62b924b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-bitfield-read-9.c
@@ -0,0 +1,51 @@
+/* { dg-require-effective-target vect_shift } */
+/* { dg-require-effective-target vect_long_long } */
+/* { dg-additional-options { "-fdump-tree-ifcvt-all" } } */
+
+#include <stdarg.h>
+#include "tree-vect.h"
+
+extern void abort(void);
+
+struct s {
+    unsigned i : 31;
+    char a : 4;
+};
+
+#define N 32
+#define ELT0 {0x7FFFFFFFUL, 0}
+#define ELT1 {0x7FFFFFFFUL, 1}
+#define ELT2 {0x7FFFFFFFUL, 2}
+#define ELT3 {0x7FFFFFFFUL, 3}
+#define ELT4 {0x7FFFFFFFUL, 4}
+#define RES 9
+struct s A[N]
+  = { ELT0, ELT4, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3,
+      ELT0, ELT1, ELT2, ELT3, ELT0, ELT1, ELT2, ELT3};
+
+int __attribute__ ((noipa))
+f(struct s *ptr, unsigned n) {
+    int res = 0;
+    for (int i = 0; i < n; ++i)
+      {
+	if (ptr[i].a)
+	  return 9;
+	res += ptr[i].a;
+      }
+    return res;
+}
+
+int main (void)
+{
+  check_vect ();
+
+  if (f(&A[0], N) != RES)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "Bitfield OK to lower." "ifcvt" } } */
+
diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
index a83b013d2ad3d9066b1c2bc62282ec054483598f..639cd91f61517b4cadd50cca253eec3ccdf79b8d 100644
--- a/gcc/tree-if-conv.cc
+++ b/gcc/tree-if-conv.cc
@@ -1257,6 +1257,31 @@ get_loop_body_in_if_conv_order (const class loop *loop)
     }
   free (blocks_in_bfs_order);
   BITMAP_FREE (visited);
+
+  /* Go through loop and reject if-conversion or lowering of bitfields if we
+     encounter statements we do not believe the vectorizer will be able to
+     handle.  If adding a new type of statement here, make sure
+     'ifcvt_local_dce' is also able to handle it propertly.  */
+  for (index = 0; index < loop->num_nodes; index++)
+    {
+      basic_block bb = blocks[index];
+      gimple_stmt_iterator gsi;
+
+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	switch (gimple_code (gsi_stmt (gsi)))
+	  {
+	  case GIMPLE_LABEL:
+	  case GIMPLE_ASSIGN:
+	  case GIMPLE_CALL:
+	  case GIMPLE_DEBUG:
+	  case GIMPLE_COND:
+	    gimple_set_uid (gsi_stmt (gsi), 0);
+	    break;
+	  default:
+	    free (blocks);
+	    return NULL;
+	  }
+    }
   return blocks;
 }
 
@@ -1429,26 +1454,6 @@ if_convertible_loop_p_1 (class loop *loop, vec<data_reference_p> *refs)
 	exit_bb = bb;
     }
 
-  for (i = 0; i < loop->num_nodes; i++)
-    {
-      basic_block bb = ifc_bbs[i];
-      gimple_stmt_iterator gsi;
-
-      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-	switch (gimple_code (gsi_stmt (gsi)))
-	  {
-	  case GIMPLE_LABEL:
-	  case GIMPLE_ASSIGN:
-	  case GIMPLE_CALL:
-	  case GIMPLE_DEBUG:
-	  case GIMPLE_COND:
-	    gimple_set_uid (gsi_stmt (gsi), 0);
-	    break;
-	  default:
-	    return false;
-	  }
-    }
-
   data_reference_p dr;
 
   innermost_DR_map
@@ -1560,14 +1565,6 @@ if_convertible_loop_p (class loop *loop, vec<data_reference_p> *refs)
       return false;
     }
 
-  /* More than one loop exit is too much to handle.  */
-  if (!single_exit (loop))
-    {
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	fprintf (dump_file, "multiple exits\n");
-      return false;
-    }
-
   /* If one of the loop header's edge is an exit edge then do not
      apply if-conversion.  */
   FOR_EACH_EDGE (e, ei, loop->header->succs)
@@ -3510,9 +3507,6 @@ tree_if_conversion (class loop *loop, vec<gimple *> *preds)
 	aggressive_if_conv = true;
     }
 
-  if (!single_exit (loop))
-    goto cleanup;
-
   /* If there are more than two BBs in the loop then there is at least one if
      to convert.  */
   if (loop->num_nodes > 2
@@ -3532,15 +3526,25 @@ tree_if_conversion (class loop *loop, vec<gimple *> *preds)
 
   if (loop->num_nodes > 2)
     {
-      need_to_ifcvt = true;
+      /* More than one loop exit is too much to handle.  */
+      if (!single_exit (loop))
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    fprintf (dump_file, "Can not ifcvt due to multiple exits\n");
+	}
+      else
+	{
+	  need_to_ifcvt = true;
 
-      if (!if_convertible_loop_p (loop, &refs) || !dbg_cnt (if_conversion_tree))
-	goto cleanup;
+	  if (!if_convertible_loop_p (loop, &refs)
+	      || !dbg_cnt (if_conversion_tree))
+	    goto cleanup;
 
-      if ((need_to_predicate || any_complicated_phi)
-	  && ((!flag_tree_loop_vectorize && !loop->force_vectorize)
-	      || loop->dont_vectorize))
-	goto cleanup;
+	  if ((need_to_predicate || any_complicated_phi)
+	      && ((!flag_tree_loop_vectorize && !loop->force_vectorize)
+		  || loop->dont_vectorize))
+	    goto cleanup;
+	}
     }
 
   if ((flag_tree_loop_vectorize || loop->force_vectorize)
@@ -3631,7 +3635,8 @@ tree_if_conversion (class loop *loop, vec<gimple *> *preds)
      PHIs, those are to be kept in sync with the non-if-converted copy.
      ???  We'll still keep dead stores though.  */
   exit_bbs = BITMAP_ALLOC (NULL);
-  bitmap_set_bit (exit_bbs, single_exit (loop)->dest->index);
+  for (edge exit : get_loop_exit_edges (loop))
+    bitmap_set_bit (exit_bbs, exit->dest->index);
   bitmap_set_bit (exit_bbs, loop->latch->index);
 
   std::pair <tree, tree> *name_pair;

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ifcvt: Support bitfield lowering of multiple-exit loops
  2022-11-03 13:43 [PATCH] ifcvt: Support bitfield lowering of multiple-exit loops Andre Vieira (lists)
@ 2022-11-07 11:05 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2022-11-07 11:05 UTC (permalink / raw)
  To: Andre Vieira (lists); +Cc: gcc-patches, Richard Sandiford

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On Thu, 3 Nov 2022, Andre Vieira (lists) wrote:

> Hi,
> 
> With Tamar's patch
> (https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604880.html) enabling
> the vectorization of early-breaks, I'd like to allow bitfield lowering in such
> loops, which requires the relaxation of allowing multiple exits when doing
> so.  In order to avoid a similar issue to PR107275, I hoisted the code that
> rejects loops with certain types of gimple_stmts from
> 'if_convertible_loop_p_1' to 'get_loop_body_in_if_conv_order', to avoid trying
> to lower bitfields in loops we are not going to vectorize anyway.  This also
> ensures 'ifcvt_local_dce' doesn't accidentally remove statements it shouldn't
> as it will never come across them.  I made sure to add a comment to make clear
> that there is a direct connection between the two and if we were to enable
> vectorization of any other gimple statement we should make sure both handle
> it.
> 
> Bootstrapped and regression tested on aarch64-none-linux-gnu and
> x86_64-pc-linux-gnu

OK.

Thanks,
Richard.

> gcc/ChangeLog:
> 
>         * tree-if-conv.cc (if_convertible_loop_p_1): Move statement check loop
> from here ...
>         (get_loop_body_if_conv_order): ... to here.
>         (if_convertible_loop_p): Remove single_exit check.
>         (tree_if_conversion): Move single_exit check to if-conversion part.
> 
> gcc/testsuite/ChangeLog:
> 
>         * gcc.dg/vect/vect-bitfield-read-1-not.c: New test.
>         * gcc.dg/vect/vect-bitfield-read-2-not.c: New test.
>         * gcc.dg/vect/vect-bitfield-read-8.c: New test.
>         * gcc.dg/vect/vect-bitfield-read-9.c: New test.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-11-07 11:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 13:43 [PATCH] ifcvt: Support bitfield lowering of multiple-exit loops Andre Vieira (lists)
2022-11-07 11:05 ` Richard Biener

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).