public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
@ 2020-03-30  9:30 Yangfei (Felix)
  0 siblings, 0 replies; 7+ messages in thread
From: Yangfei (Felix) @ 2020-03-30  9:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: rguenther

Hi,

New bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94398 

With -mstrict-align, aarch64_builtin_support_vector_misalignment will returns false when misalignment factor is unknown at compile time.
Then vect_supportable_dr_alignment returns dr_unaligned_unsupported, which triggers the ICE.  I have pasted the call trace on the bug report.

vect_supportable_dr_alignment is expected to return either dr_aligned or dr_unaligned_supported for masked operations.
But it seems that this function only catches internal_fn IFN_MASK_LOAD & IFN_MASK_STORE.
We are emitting a mask gather load here for this test case.
As backends have their own vector misalignment support policy, I am supposing this should be better handled in the auto-vect shared code.

Proposed fix:
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 0192aa6..67d3345 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -6509,11 +6509,26 @@ vect_supportable_dr_alignment (dr_vec_info *dr_info,

   /* For now assume all conditional loads/stores support unaligned
      access without any special code.  */
-  if (gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt))
-    if (gimple_call_internal_p (stmt)
-       && (gimple_call_internal_fn (stmt) == IFN_MASK_LOAD
-           || gimple_call_internal_fn (stmt) == IFN_MASK_STORE))
-      return dr_unaligned_supported;
+  gcall *call = dyn_cast <gcall *> (stmt_info->stmt);
+  if (call && gimple_call_internal_p (call))
+    {
+      internal_fn ifn = gimple_call_internal_fn (call);
+      switch (ifn)
+       {
+         case IFN_MASK_LOAD:
+         case IFN_MASK_LOAD_LANES:
+         case IFN_MASK_GATHER_LOAD:
+         case IFN_MASK_STORE:
+         case IFN_MASK_STORE_LANES:
+         case IFN_MASK_SCATTER_STORE:
+           return dr_unaligned_supported;
+         default:
+           break;
+       }
+    }
+
+  if (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
+    return dr_unaligned_supported;

   if (loop_vinfo)
     {

Suggestions?

Thanks,
Felix

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

* Re: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
  2020-03-31 11:35       ` Yangfei (Felix)
@ 2020-03-31 14:19         ` Richard Sandiford
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Sandiford @ 2020-03-31 14:19 UTC (permalink / raw)
  To: Yangfei (Felix); +Cc: gcc-patches, rguenther

"Yangfei (Felix)" <felix.yang@huawei.com> writes:
>> > I think I need a sponsor if this patch can go separately.
>> 
>> Yeah, please fill in the form on:
>> 
>>    https://sourceware.org/cgi-bin/pdw/ps_form.cgi
>> 
>> listing me as sponsor.
>
> Hmm, I already have an account : - )  

Oops, yes.  I should have checked, sorry.

> But my networking does not work well and I am having some trouble committing the patch.  
> Could you please help commit this patch?  

OK, pushed to master.

Thanks,
Richard

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

* RE: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
  2020-03-31  8:54     ` Richard Sandiford
@ 2020-03-31 11:35       ` Yangfei (Felix)
  2020-03-31 14:19         ` Richard Sandiford
  0 siblings, 1 reply; 7+ messages in thread
From: Yangfei (Felix) @ 2020-03-31 11:35 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches, rguenther

Hi!

> -----Original Message-----
> From: Richard Sandiford [mailto:richard.sandiford@arm.com]
> Sent: Tuesday, March 31, 2020 4:55 PM
> To: Yangfei (Felix) <felix.yang@huawei.com>
> Cc: gcc-patches@gcc.gnu.org; rguenther@suse.de
> Subject: Re: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
> >
> > Yes, I have modified accordingly.  Attached please find the adapted patch.
> > Bootstrapped and tested on aarch64-linux-gnu.  Newly add test will fail
> without the patch and pass otherwise.
> 
> Looks good.  OK for master.

Thanks for reviewing this.  

> > I think I need a sponsor if this patch can go separately.
> 
> Yeah, please fill in the form on:
> 
>    https://sourceware.org/cgi-bin/pdw/ps_form.cgi
> 
> listing me as sponsor.

Hmm, I already have an account : - )  
But my networking does not work well and I am having some trouble committing the patch.  
Could you please help commit this patch?  

Felix

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

* Re: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
  2020-03-31  8:46   ` Yangfei (Felix)
@ 2020-03-31  8:54     ` Richard Sandiford
  2020-03-31 11:35       ` Yangfei (Felix)
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2020-03-31  8:54 UTC (permalink / raw)
  To: Yangfei (Felix); +Cc: gcc-patches, rguenther

"Yangfei (Felix)" <felix.yang@huawei.com> writes:
> Hi!
>
>> -----Original Message-----
>> From: Richard Sandiford [mailto:richard.sandiford@arm.com]
>> Sent: Monday, March 30, 2020 8:08 PM
>> To: Yangfei (Felix) <felix.yang@huawei.com>
>> Cc: gcc-patches@gcc.gnu.org; rguenther@suse.de
>> Subject: Re: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
>> 
>> "Yangfei (Felix)" <felix.yang@huawei.com> writes:
>> > Hi!
>> >> -----Original Message-----
>> >> From: Yangfei (Felix)
>> >> Sent: Monday, March 30, 2020 5:28 PM
>> >> To: gcc-patches@gcc.gnu.org
>> >> Cc: 'rguenther@suse.de' <rguenther@suse.de>
>> >> Subject: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
>> >>
>> >> Hi,
>> >>
>> >> New bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94398
>> >>
>> >> With -mstrict-align, aarch64_builtin_support_vector_misalignment will
>> >> returns false when misalignment factor is unknown at compile time.
>> >> Then vect_supportable_dr_alignment returns dr_unaligned_unsupported,
>> >> which triggers the ICE.  I have pasted the call trace on the bug report.
>> >>
>> >> vect_supportable_dr_alignment is expected to return either dr_aligned
>> >> or dr_unaligned_supported for masked operations.
>> >> But it seems that this function only catches internal_fn
>> >> IFN_MASK_LOAD & IFN_MASK_STORE.
>> >> We are emitting a mask gather load here for this test case.
>> >> As backends have their own vector misalignment support policy, I am
>> >> supposing this should be better handled in the auto-vect shared code.
>> >>
>> >
>> > I can only reply to comment on the bug here as my account got locked by the
>> bugzilla system for now.
>> 
>> Sorry to hear that.  What was the reason?
>
> Looks like it got filtered by spamassassin.  Admin has helped unlocked it.  
>
>> > The way Richard (rsandifo) suggests also works for me and I agree it should
>> be more consistent and better for compile time.
>> > One question here: when will a IFN_MASK_LOAD/IFN_MASK_STORE be
>> passed to vect_supportable_dr_alignment?
>> 
>> I'd expect that to happen in the same cases as for unmasked load/stores.
>> It certainly will for unconditional loads and stores that get masked via full-loop
>> masking.
>> 
>> In principle, the same rules apply to both masked and unmasked accesses.
>> But for SVE, both masked and unmasked accesses should support misalignment,
>> which is why I think the current target hook is probably wrong for SVE +
>> -mstrict-align.
>
> I stopped looking into the backend further when I saw no distinction for different type of access
> in the target hook aarch64_builtin_support_vector_misalignment. 
>
>> > @@ -8051,8 +8051,15 @@ vectorizable_store (stmt_vec_info stmt_info,
>> gimple_stmt_iterator *gsi,
>> >    auto_vec<tree> dr_chain (group_size);
>> >    oprnds.create (group_size);
>> >
>> > -  alignment_support_scheme
>> > -    = vect_supportable_dr_alignment (first_dr_info, false);
>> > +  /* Strided accesses perform only component accesses, alignment
>> > +     is irrelevant for them.  */
>> > +  if (STMT_VINFO_STRIDED_P (first_dr_info->stmt)
>> > +      && !STMT_VINFO_GROUPED_ACCESS (first_dr_info->stmt))
>> 
>> I think this should be based on memory_access_type ==
>> VMAT_GATHER_SCATTER instead.  At this point, STMT_VINFO_* describes
>> properties of the original scalar access(es) while memory_access_type
>> describes the vector implementation strategy.  It's the latter that matters
>> here.
>> 
>> Same thing for loads.
>
> Yes, I have modified accordingly.  Attached please find the adapted patch.  
> Bootstrapped and tested on aarch64-linux-gnu.  Newly add test will fail without the patch and pass otherwise.  

Looks good.  OK for master.

> I think I need a sponsor if this patch can go separately.  

Yeah, please fill in the form on:

   https://sourceware.org/cgi-bin/pdw/ps_form.cgi

listing me as sponsor.

Thanks,
Richard

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

* RE: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
  2020-03-30 12:08 ` Richard Sandiford
@ 2020-03-31  8:46   ` Yangfei (Felix)
  2020-03-31  8:54     ` Richard Sandiford
  0 siblings, 1 reply; 7+ messages in thread
From: Yangfei (Felix) @ 2020-03-31  8:46 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches, rguenther

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

Hi!

> -----Original Message-----
> From: Richard Sandiford [mailto:richard.sandiford@arm.com]
> Sent: Monday, March 30, 2020 8:08 PM
> To: Yangfei (Felix) <felix.yang@huawei.com>
> Cc: gcc-patches@gcc.gnu.org; rguenther@suse.de
> Subject: Re: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
> 
> "Yangfei (Felix)" <felix.yang@huawei.com> writes:
> > Hi!
> >> -----Original Message-----
> >> From: Yangfei (Felix)
> >> Sent: Monday, March 30, 2020 5:28 PM
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: 'rguenther@suse.de' <rguenther@suse.de>
> >> Subject: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
> >>
> >> Hi,
> >>
> >> New bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94398
> >>
> >> With -mstrict-align, aarch64_builtin_support_vector_misalignment will
> >> returns false when misalignment factor is unknown at compile time.
> >> Then vect_supportable_dr_alignment returns dr_unaligned_unsupported,
> >> which triggers the ICE.  I have pasted the call trace on the bug report.
> >>
> >> vect_supportable_dr_alignment is expected to return either dr_aligned
> >> or dr_unaligned_supported for masked operations.
> >> But it seems that this function only catches internal_fn
> >> IFN_MASK_LOAD & IFN_MASK_STORE.
> >> We are emitting a mask gather load here for this test case.
> >> As backends have their own vector misalignment support policy, I am
> >> supposing this should be better handled in the auto-vect shared code.
> >>
> >
> > I can only reply to comment on the bug here as my account got locked by the
> bugzilla system for now.
> 
> Sorry to hear that.  What was the reason?

Looks like it got filtered by spamassassin.  Admin has helped unlocked it.  

> > The way Richard (rsandifo) suggests also works for me and I agree it should
> be more consistent and better for compile time.
> > One question here: when will a IFN_MASK_LOAD/IFN_MASK_STORE be
> passed to vect_supportable_dr_alignment?
> 
> I'd expect that to happen in the same cases as for unmasked load/stores.
> It certainly will for unconditional loads and stores that get masked via full-loop
> masking.
> 
> In principle, the same rules apply to both masked and unmasked accesses.
> But for SVE, both masked and unmasked accesses should support misalignment,
> which is why I think the current target hook is probably wrong for SVE +
> -mstrict-align.

I stopped looking into the backend further when I saw no distinction for different type of access
in the target hook aarch64_builtin_support_vector_misalignment. 

> > @@ -8051,8 +8051,15 @@ vectorizable_store (stmt_vec_info stmt_info,
> gimple_stmt_iterator *gsi,
> >    auto_vec<tree> dr_chain (group_size);
> >    oprnds.create (group_size);
> >
> > -  alignment_support_scheme
> > -    = vect_supportable_dr_alignment (first_dr_info, false);
> > +  /* Strided accesses perform only component accesses, alignment
> > +     is irrelevant for them.  */
> > +  if (STMT_VINFO_STRIDED_P (first_dr_info->stmt)
> > +      && !STMT_VINFO_GROUPED_ACCESS (first_dr_info->stmt))
> 
> I think this should be based on memory_access_type ==
> VMAT_GATHER_SCATTER instead.  At this point, STMT_VINFO_* describes
> properties of the original scalar access(es) while memory_access_type
> describes the vector implementation strategy.  It's the latter that matters
> here.
> 
> Same thing for loads.

Yes, I have modified accordingly.  Attached please find the adapted patch.  
Bootstrapped and tested on aarch64-linux-gnu.  Newly add test will fail without the patch and pass otherwise.  
I think I need a sponsor if this patch can go separately.  

Thanks,
Felix

[-- Attachment #2: pr94398-v1.patch --]
[-- Type: application/octet-stream, Size: 4612 bytes --]

From 66ff513e39b8d87b83e0a15d71f12b088374f3cd Mon Sep 17 00:00:00 2001
From: Felix Yang <felix.yang@huawei.com>
Date: Tue, 31 Mar 2020 16:41:56 +0800
Subject: [PATCH] vect: ICE: in vectorizable_load, at tree-vect-stmts.c:9173
 [PR94398]

In the testcase for PR94398, we're trying to compute:

  alignment_support_scheme
    = vect_supportable_dr_alignment (first_dr_info, false);
  gcc_assert (alignment_support_scheme);

even for VMAT_GATHER_SCATTER, which always accesses individual elements.
Here we should set alignment_support_scheme to dr_unaligned_supported
the gather/scatter case instead of calling vect_supportable_dr_alignment.

    2020-03-31  Felix Yang  <felix.yang@huawei.com>

    gcc/
    PR tree-optimization/94398
    * tree-vect-stmts.c (vectorizable_store): Instead of calling
    vect_supportable_dr_alignment, set alignment_support_scheme to
    dr_unaligned_supported for gather-scatter accesses.
    (vectorizable_load): Likewise.

    gcc/testsuite/
    PR tree-optimization/94398
    * gcc.target/aarch64/pr94398.c: New test.
---
 gcc/ChangeLog                              |  8 ++++++++
 gcc/testsuite/ChangeLog                    |  5 +++++
 gcc/testsuite/gcc.target/aarch64/pr94398.c | 24 ++++++++++++++++++++++++
 gcc/tree-vect-stmts.c                      | 20 ++++++++++++++++----
 4 files changed, 53 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94398.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 101956a..5b8844b 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-03-31  Felix Yang  <felix.yang@huawei.com>
+
+	PR tree-optimization/94398
+	* tree-vect-stmts.c (vectorizable_store): Instead of calling
+	vect_supportable_dr_alignment, set alignment_support_scheme to
+	dr_unaligned_supported for gather-scatter accesses.
+	(vectorizable_load): Likewise.
+
 2020-03-30  David Malcolm  <dmalcolm@redhat.com>
 
 	* lra.c (finish_insn_code_data_once): Set the array elements
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index c72aa9a..23e43a5 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-31  Felix Yang  <felix.yang@huawei.com>
+
+	PR tree-optimization/94398
+	* gcc.target/aarch64/pr94398.c: New test.
+
 2020-03-30  David Malcolm  <dmalcolm@redhat.com>
 
 	* jit.dg/all-non-failing-tests.h: Add test-empty.c
diff --git a/gcc/testsuite/gcc.target/aarch64/pr94398.c b/gcc/testsuite/gcc.target/aarch64/pr94398.c
new file mode 100644
index 0000000..42152cf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr94398.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-loop-vectorize -funsafe-math-optimizations -march=armv8.2-a+sve -mstrict-align" } */
+
+float
+foo(long n, float *x, int inc_x,
+            float *y, int inc_y)
+{
+  float dot = 0.0;
+  int ix = 0, iy = 0;
+
+  if (n < 0) {
+    return dot;
+  }
+
+  int i = 0;
+  while (i < n) {
+    dot += y[iy] * x[ix];
+    ix  += inc_x;
+    iy  += inc_y;
+    i++;
+  }
+
+  return dot;
+}
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 12beef6..46bc2bd 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -8051,8 +8051,14 @@ vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
   auto_vec<tree> dr_chain (group_size);
   oprnds.create (group_size);
 
-  alignment_support_scheme
-    = vect_supportable_dr_alignment (first_dr_info, false);
+  /* Gather-scatter accesses perform only component accesses, alignment
+     is irrelevant for them.  */
+  if (memory_access_type == VMAT_GATHER_SCATTER)
+    alignment_support_scheme = dr_unaligned_supported;
+  else
+    alignment_support_scheme
+      = vect_supportable_dr_alignment (first_dr_info, false);
+
   gcc_assert (alignment_support_scheme);
   vec_loop_masks *loop_masks
     = (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)
@@ -9168,8 +9174,14 @@ vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
       ref_type = reference_alias_ptr_type (DR_REF (first_dr_info->dr));
     }
 
-  alignment_support_scheme
-    = vect_supportable_dr_alignment (first_dr_info, false);
+  /* Gather-scatter accesses perform only component accesses, alignment
+     is irrelevant for them.  */
+  if (memory_access_type == VMAT_GATHER_SCATTER)
+    alignment_support_scheme = dr_unaligned_supported;
+  else
+    alignment_support_scheme
+      = vect_supportable_dr_alignment (first_dr_info, false);
+
   gcc_assert (alignment_support_scheme);
   vec_loop_masks *loop_masks
     = (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)
-- 
1.8.3.1


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

* Re: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
  2020-03-30 11:53 Yangfei (Felix)
@ 2020-03-30 12:08 ` Richard Sandiford
  2020-03-31  8:46   ` Yangfei (Felix)
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2020-03-30 12:08 UTC (permalink / raw)
  To: Yangfei (Felix); +Cc: gcc-patches, rguenther

"Yangfei (Felix)" <felix.yang@huawei.com> writes:
> Hi!
>> -----Original Message-----
>> From: Yangfei (Felix)
>> Sent: Monday, March 30, 2020 5:28 PM
>> To: gcc-patches@gcc.gnu.org
>> Cc: 'rguenther@suse.de' <rguenther@suse.de>
>> Subject: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
>> 
>> Hi,
>> 
>> New bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94398
>> 
>> With -mstrict-align, aarch64_builtin_support_vector_misalignment will returns
>> false when misalignment factor is unknown at compile time.
>> Then vect_supportable_dr_alignment returns dr_unaligned_unsupported,
>> which triggers the ICE.  I have pasted the call trace on the bug report.
>> 
>> vect_supportable_dr_alignment is expected to return either dr_aligned or
>> dr_unaligned_supported for masked operations.
>> But it seems that this function only catches internal_fn IFN_MASK_LOAD &
>> IFN_MASK_STORE.
>> We are emitting a mask gather load here for this test case.
>> As backends have their own vector misalignment support policy, I am supposing
>> this should be better handled in the auto-vect shared code.
>> 
>
> I can only reply to comment on the bug here as my account got locked by the bugzilla system for now. 

Sorry to hear that.  What was the reason?

> The way Richard (rsandifo) suggests also works for me and I agree it should be more consistent and better for compile time. 
> One question here: when will a IFN_MASK_LOAD/IFN_MASK_STORE be passed to vect_supportable_dr_alignment? 

I'd expect that to happen in the same cases as for unmasked load/stores.
It certainly will for unconditional loads and stores that get masked
via full-loop masking.

In principle, the same rules apply to both masked and unmasked accesses.
But for SVE, both masked and unmasked accesses should support misalignment,
which is why I think the current target hook is probably wrong for
SVE + -mstrict-align.

> @@ -8051,8 +8051,15 @@ vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
>    auto_vec<tree> dr_chain (group_size);
>    oprnds.create (group_size);
>
> -  alignment_support_scheme
> -    = vect_supportable_dr_alignment (first_dr_info, false);
> +  /* Strided accesses perform only component accesses, alignment
> +     is irrelevant for them.  */
> +  if (STMT_VINFO_STRIDED_P (first_dr_info->stmt)
> +      && !STMT_VINFO_GROUPED_ACCESS (first_dr_info->stmt))

I think this should be based on memory_access_type == VMAT_GATHER_SCATTER
instead.  At this point, STMT_VINFO_* describes properties of the original
scalar access(es) while memory_access_type describes the vector
implementation strategy.  It's the latter that matters here.

Same thing for loads.

Thanks,
Richard

> +    alignment_support_scheme = dr_unaligned_supported;
> +  else
> +    alignment_support_scheme
> +      = vect_supportable_dr_alignment (first_dr_info, false);
> +
>    gcc_assert (alignment_support_scheme);
>    vec_loop_masks *loop_masks
>      = (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)
> @@ -9168,8 +9175,15 @@ vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
>        ref_type = reference_alias_ptr_type (DR_REF (first_dr_info->dr));
>      }
>
> -  alignment_support_scheme
> -    = vect_supportable_dr_alignment (first_dr_info, false);
> +  /* Strided accesses perform only component accesses, alignment
> +     is irrelevant for them.  */
> +  if (STMT_VINFO_STRIDED_P (first_dr_info->stmt)
> +      && !STMT_VINFO_GROUPED_ACCESS (first_dr_info->stmt))
> +    alignment_support_scheme = dr_unaligned_supported;
> +  else
> +    alignment_support_scheme
> +      = vect_supportable_dr_alignment (first_dr_info, false);
> +
>    gcc_assert (alignment_support_scheme);
>    vec_loop_masks *loop_masks
>      = (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)

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

* RE: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
@ 2020-03-30 11:53 Yangfei (Felix)
  2020-03-30 12:08 ` Richard Sandiford
  0 siblings, 1 reply; 7+ messages in thread
From: Yangfei (Felix) @ 2020-03-30 11:53 UTC (permalink / raw)
  To: gcc-patches; +Cc: rguenther, richard.sandiford

Hi!
> -----Original Message-----
> From: Yangfei (Felix)
> Sent: Monday, March 30, 2020 5:28 PM
> To: gcc-patches@gcc.gnu.org
> Cc: 'rguenther@suse.de' <rguenther@suse.de>
> Subject: [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173
> 
> Hi,
> 
> New bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94398
> 
> With -mstrict-align, aarch64_builtin_support_vector_misalignment will returns
> false when misalignment factor is unknown at compile time.
> Then vect_supportable_dr_alignment returns dr_unaligned_unsupported,
> which triggers the ICE.  I have pasted the call trace on the bug report.
> 
> vect_supportable_dr_alignment is expected to return either dr_aligned or
> dr_unaligned_supported for masked operations.
> But it seems that this function only catches internal_fn IFN_MASK_LOAD &
> IFN_MASK_STORE.
> We are emitting a mask gather load here for this test case.
> As backends have their own vector misalignment support policy, I am supposing
> this should be better handled in the auto-vect shared code.
> 

I can only reply to comment on the bug here as my account got locked by the bugzilla system for now. 
The way Richard (rsandifo) suggests also works for me and I agree it should be more consistent and better for compile time. 
One question here: when will a IFN_MASK_LOAD/IFN_MASK_STORE be passed to vect_supportable_dr_alignment? 

New patch:
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 12beef6..2825023 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -8051,8 +8051,15 @@ vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
   auto_vec<tree> dr_chain (group_size);
   oprnds.create (group_size);

-  alignment_support_scheme
-    = vect_supportable_dr_alignment (first_dr_info, false);
+  /* Strided accesses perform only component accesses, alignment
+     is irrelevant for them.  */
+  if (STMT_VINFO_STRIDED_P (first_dr_info->stmt)
+      && !STMT_VINFO_GROUPED_ACCESS (first_dr_info->stmt))
+    alignment_support_scheme = dr_unaligned_supported;
+  else
+    alignment_support_scheme
+      = vect_supportable_dr_alignment (first_dr_info, false);
+
   gcc_assert (alignment_support_scheme);
   vec_loop_masks *loop_masks
     = (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)
@@ -9168,8 +9175,15 @@ vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
       ref_type = reference_alias_ptr_type (DR_REF (first_dr_info->dr));
     }

-  alignment_support_scheme
-    = vect_supportable_dr_alignment (first_dr_info, false);
+  /* Strided accesses perform only component accesses, alignment
+     is irrelevant for them.  */
+  if (STMT_VINFO_STRIDED_P (first_dr_info->stmt)
+      && !STMT_VINFO_GROUPED_ACCESS (first_dr_info->stmt))
+    alignment_support_scheme = dr_unaligned_supported;
+  else
+    alignment_support_scheme
+      = vect_supportable_dr_alignment (first_dr_info, false);
+
   gcc_assert (alignment_support_scheme);
   vec_loop_masks *loop_masks
     = (loop_vinfo && LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)

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

end of thread, other threads:[~2020-03-31 14:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30  9:30 [PATCH] ICE: in vectorizable_load, at tree-vect-stmts.c:9173 Yangfei (Felix)
2020-03-30 11:53 Yangfei (Felix)
2020-03-30 12:08 ` Richard Sandiford
2020-03-31  8:46   ` Yangfei (Felix)
2020-03-31  8:54     ` Richard Sandiford
2020-03-31 11:35       ` Yangfei (Felix)
2020-03-31 14:19         ` Richard Sandiford

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