diff mbox series

tree-vect-generic: Fix bitfield widths [PR94980 3/3]

Message ID mptlflya0fd.fsf@arm.com
State New
Headers show
Series tree-vect-generic: Fix bitfield widths [PR94980 3/3] | expand

Commit Message

Richard Sandiford May 11, 2020, 4:41 p.m. UTC
This third patch of three actually fixes the PR.  We were using
8-bit BIT_FIELD_REFs to access single-bit elements, and multiplying
the vector index by 8 bits rather than 1 bit.

Tested individually on aarch64-linux-gnu and as a series on
x86_64-linux-gnu.  OK to install?

I'm not sure what to do about backports.  Arguably the PR is
really a progression rather than a regression, since we went
from generating wrong code to ICEing.  I'm not at all convinced
that this fixes all the vector-lowering problems associated
with packed booleans, and there's a danger that we could regress
to wrong code if we backported a piecemeal fix.

Richard


2020-05-11  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR tree-optimization/94980
	* tree-vect-generic.c (expand_vector_comparison): Use
	vector_element_bits_tree to get the element size in bits,
	rather than using TYPE_SIZE.
	(expand_vector_condition, vector_element): Likewise.

gcc/testsuite/
	PR tree-optimization/94980
	* gcc.target/i386/pr94980.c: New test.
---
 gcc/testsuite/gcc.target/i386/pr94980.c | 10 ++++++++++
 gcc/tree-vect-generic.c                 |  8 ++++----
 2 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr94980.c

Comments

Richard Biener May 11, 2020, 6:21 p.m. UTC | #1
On May 11, 2020 6:41:58 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>This third patch of three actually fixes the PR.  We were using
>8-bit BIT_FIELD_REFs to access single-bit elements, and multiplying
>the vector index by 8 bits rather than 1 bit.
>
>Tested individually on aarch64-linux-gnu and as a series on
>x86_64-linux-gnu.  OK to install?

OK. 

>I'm not sure what to do about backports.  Arguably the PR is
>really a progression rather than a regression, since we went
>from generating wrong code to ICEing.  I'm not at all convinced
>that this fixes all the vector-lowering problems associated
>with packed booleans, and there's a danger that we could regress
>to wrong code if we backported a piecemeal fix.

I would suggest to wait for a real world case and only backport once we're sufficiently sure the support is complete. 

Richard 

>Richard
>
>
>2020-05-11  Richard Sandiford  <richard.sandiford@arm.com>
>
>gcc/
>	PR tree-optimization/94980
>	* tree-vect-generic.c (expand_vector_comparison): Use
>	vector_element_bits_tree to get the element size in bits,
>	rather than using TYPE_SIZE.
>	(expand_vector_condition, vector_element): Likewise.
>
>gcc/testsuite/
>	PR tree-optimization/94980
>	* gcc.target/i386/pr94980.c: New test.
>---
> gcc/testsuite/gcc.target/i386/pr94980.c | 10 ++++++++++
> gcc/tree-vect-generic.c                 |  8 ++++----
> 2 files changed, 14 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr94980.c
>
>diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
>index adea9337a97..a7fe83da0e3 100644
>--- a/gcc/tree-vect-generic.c
>+++ b/gcc/tree-vect-generic.c
>@@ -390,7 +390,7 @@ expand_vector_comparison (gimple_stmt_iterator
>*gsi, tree type, tree op0,
> 						(TREE_TYPE (type)))))
> 	{
> 	  tree inner_type = TREE_TYPE (TREE_TYPE (op0));
>-	  tree part_width = TYPE_SIZE (inner_type);
>+	  tree part_width = vector_element_bits_tree (TREE_TYPE (op0));
> 	  tree index = bitsize_int (0);
> 	  int nunits = nunits_for_known_piecewise_op (TREE_TYPE (op0));
> 	  int prec = GET_MODE_PRECISION (SCALAR_TYPE_MODE (type));
>@@ -944,9 +944,9 @@ expand_vector_condition (gimple_stmt_iterator *gsi)
>   vec<constructor_elt, va_gc> *v;
>   tree constr;
>   tree inner_type = TREE_TYPE (type);
>+  tree width = vector_element_bits_tree (type);
>   tree cond_type = TREE_TYPE (TREE_TYPE (a));
>   tree comp_inner_type = cond_type;
>-  tree width = TYPE_SIZE (inner_type);
>   tree index = bitsize_int (0);
>   tree comp_width = width;
>   tree comp_index = index;
>@@ -960,7 +960,7 @@ expand_vector_condition (gimple_stmt_iterator *gsi)
>       a1 = TREE_OPERAND (a, 0);
>       a2 = TREE_OPERAND (a, 1);
>       comp_inner_type = TREE_TYPE (TREE_TYPE (a1));
>-      comp_width = TYPE_SIZE (comp_inner_type);
>+      comp_width = vector_element_bits_tree (TREE_TYPE (a1));
>     }
> 
>   if (expand_vec_cond_expr_p (type, TREE_TYPE (a1), TREE_CODE (a)))
>@@ -1333,7 +1333,7 @@ vector_element (gimple_stmt_iterator *gsi, tree
>vect, tree idx, tree *ptmpvec)
>         }
>       else
>         {
>-	  tree size = TYPE_SIZE (vect_elt_type);
>+	  tree size = vector_element_bits_tree (vect_type);
>	  tree pos = fold_build2 (MULT_EXPR, bitsizetype, bitsize_int (index),
> 				  size);
> 	  return fold_build3 (BIT_FIELD_REF, vect_elt_type, vect, size, pos);
>diff --git a/gcc/testsuite/gcc.target/i386/pr94980.c
>b/gcc/testsuite/gcc.target/i386/pr94980.c
>new file mode 100644
>index 00000000000..488f94abec9
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/i386/pr94980.c
>@@ -0,0 +1,10 @@
>+/* { dg-do compile } */
>+/* { dg-options "-mavx512vl" } */
>+
>+int __attribute__((__vector_size__(16))) v;
>+
>+void
>+foo(void)
>+{
>+  0 <= (0 != v) >= 0;
>+}
diff mbox series

Patch

diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index adea9337a97..a7fe83da0e3 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -390,7 +390,7 @@  expand_vector_comparison (gimple_stmt_iterator *gsi, tree type, tree op0,
 						(TREE_TYPE (type)))))
 	{
 	  tree inner_type = TREE_TYPE (TREE_TYPE (op0));
-	  tree part_width = TYPE_SIZE (inner_type);
+	  tree part_width = vector_element_bits_tree (TREE_TYPE (op0));
 	  tree index = bitsize_int (0);
 	  int nunits = nunits_for_known_piecewise_op (TREE_TYPE (op0));
 	  int prec = GET_MODE_PRECISION (SCALAR_TYPE_MODE (type));
@@ -944,9 +944,9 @@  expand_vector_condition (gimple_stmt_iterator *gsi)
   vec<constructor_elt, va_gc> *v;
   tree constr;
   tree inner_type = TREE_TYPE (type);
+  tree width = vector_element_bits_tree (type);
   tree cond_type = TREE_TYPE (TREE_TYPE (a));
   tree comp_inner_type = cond_type;
-  tree width = TYPE_SIZE (inner_type);
   tree index = bitsize_int (0);
   tree comp_width = width;
   tree comp_index = index;
@@ -960,7 +960,7 @@  expand_vector_condition (gimple_stmt_iterator *gsi)
       a1 = TREE_OPERAND (a, 0);
       a2 = TREE_OPERAND (a, 1);
       comp_inner_type = TREE_TYPE (TREE_TYPE (a1));
-      comp_width = TYPE_SIZE (comp_inner_type);
+      comp_width = vector_element_bits_tree (TREE_TYPE (a1));
     }
 
   if (expand_vec_cond_expr_p (type, TREE_TYPE (a1), TREE_CODE (a)))
@@ -1333,7 +1333,7 @@  vector_element (gimple_stmt_iterator *gsi, tree vect, tree idx, tree *ptmpvec)
         }
       else
         {
-	  tree size = TYPE_SIZE (vect_elt_type);
+	  tree size = vector_element_bits_tree (vect_type);
 	  tree pos = fold_build2 (MULT_EXPR, bitsizetype, bitsize_int (index),
 				  size);
 	  return fold_build3 (BIT_FIELD_REF, vect_elt_type, vect, size, pos);
diff --git a/gcc/testsuite/gcc.target/i386/pr94980.c b/gcc/testsuite/gcc.target/i386/pr94980.c
new file mode 100644
index 00000000000..488f94abec9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr94980.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mavx512vl" } */
+
+int __attribute__((__vector_size__(16))) v;
+
+void
+foo(void)
+{
+  0 <= (0 != v) >= 0;
+}