[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 Kewen Lin changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #11 from Kewen Lin --- Should be fixed on trunk.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 --- Comment #10 from CVS Commits --- The master branch has been updated by Kewen Lin : https://gcc.gnu.org/g:9890d4e8bcda1f34b8eefb481935ef0e4cd8069e commit r14-2813-g9890d4e8bcda1f34b8eefb481935ef0e4cd8069e Author: Kewen Lin Date: Wed Jul 26 21:43:09 2023 -0500 vect: Treat VMAT_ELEMENTWISE as scalar load in costing [PR110776] PR110776 exposes one issue that we could query unaligned load for vector type but actually no unaligned vector load is supported there. The reason is that the costed load is with single-lane vector type and its memory access type is VMAT_ELEMENTWISE, we actually take it as scalar load and set its alignment_support_scheme as dr_unaligned_supported. To avoid the ICE as exposed, following Rich's suggestion, this patch is to make VMAT_ELEMENTWISE be costed as scalar load. Co-authored-by: Richard Biener PR tree-optimization/110776 gcc/ChangeLog: * tree-vect-stmts.cc (vectorizable_load): Always cost VMAT_ELEMENTWISE as scalar load. gcc/testsuite/ChangeLog: * gcc.target/powerpc/pr110776.c: New test.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776
--- Comment #9 from Kewen Lin ---
(In reply to Iain Sandoe from comment #8)
> (In reply to rguent...@suse.de from comment #7)
> > On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote:
> >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776
> > >
> > > --- Comment #6 from Kewen Lin ---
> > > (In reply to rguent...@suse.de from comment #5)
> > > > On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote:
> > > >
> > > > I think apart from the consideration what a single element vector
> > > > is compared to a scalar, a more to-the-point fix is
> > > >
> > > > if (VECTOR_TYPE_P (ltype)
> > > > && memory_access_type != VMAT_ELEMENTWISE)
> > >
> > > Thanks for the suggestion! I thought checking lnel can also cover
> > > VMAT_STRIDED_SLP's special case having const_nunits 1, but it seems
> > > impossible
> > > to have?
> >
> > I think so, unless I'm convinced with a testcase ;)
I guess there is no such test case. ;)
>
> (sorry for being a bit slow - we had a power outage that wasted most of the
> day)
>
> Richi's suggested patch fixes build of a cross-build for powerpc-darwin and
> the test results look OK too. A non-expert look at the code suggests that
> VMAT_ELEMENTWISE is already accounted for on the write side, so that we
> should not see a call to the costing code for the equivalent write-side.
Thanks Iain, I also bootstrapped and regtested the suggested fix on x86 and
powerpc64{,le}, just posted it for review at
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625484.html.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 --- Comment #8 from Iain Sandoe --- (In reply to rguent...@suse.de from comment #7) > On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 > > > > --- Comment #6 from Kewen Lin --- > > (In reply to rguent...@suse.de from comment #5) > > > On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote: > > > > > > I think apart from the consideration what a single element vector > > > is compared to a scalar, a more to-the-point fix is > > > > > > if (VECTOR_TYPE_P (ltype) > > > && memory_access_type != VMAT_ELEMENTWISE) > > > > Thanks for the suggestion! I thought checking lnel can also cover > > VMAT_STRIDED_SLP's special case having const_nunits 1, but it seems > > impossible > > to have? > > I think so, unless I'm convinced with a testcase ;) (sorry for being a bit slow - we had a power outage that wasted most of the day) Richi's suggested patch fixes build of a cross-build for powerpc-darwin and the test results look OK too. A non-expert look at the code suggests that VMAT_ELEMENTWISE is already accounted for on the write side, so that we should not see a call to the costing code for the equivalent write-side.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 --- Comment #7 from rguenther at suse dot de --- On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 > > --- Comment #6 from Kewen Lin --- > (In reply to rguent...@suse.de from comment #5) > > On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote: > > > > I think apart from the consideration what a single element vector > > is compared to a scalar, a more to-the-point fix is > > > > if (VECTOR_TYPE_P (ltype) > > && memory_access_type != VMAT_ELEMENTWISE) > > Thanks for the suggestion! I thought checking lnel can also cover > VMAT_STRIDED_SLP's special case having const_nunits 1, but it seems impossible > to have? I think so, unless I'm convinced with a testcase ;)
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 --- Comment #6 from Kewen Lin --- (In reply to rguent...@suse.de from comment #5) > On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote: > > I think apart from the consideration what a single element vector > is compared to a scalar, a more to-the-point fix is > > if (VECTOR_TYPE_P (ltype) > && memory_access_type != VMAT_ELEMENTWISE) Thanks for the suggestion! I thought checking lnel can also cover VMAT_STRIDED_SLP's special case having const_nunits 1, but it seems impossible to have? Then it's more clear with explicit VMAT_ELEMENTWISE checking.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776
--- Comment #5 from rguenther at suse dot de ---
On Tue, 25 Jul 2023, linkw at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776
>
> Kewen Lin changed:
>
>What|Removed |Added
>
> CC||rguenth at gcc dot gnu.org
>
> --- Comment #3 from Kewen Lin ---
> The failure exposed one special case: there is one stmt
>
> # VUSE <.MEM_404>
> _15 = *_14;
>
> its STMT_VINFO_STRIDED_P (stmt_info) is set, memory_access_type is
> VMAT_ELEMENTWISE and alignment_support_scheme is dr_unaligned_supported, its
> vector type is "vector(1) long int", so in the handling it's taken as:
>
> /* Load vector(1) scalar_type if it's 1 element-wise vectype. */
> else if (nloads == 1)
> ltype = vectype;
>
> as its ltype is vector type, we cost it with
>
> if (VECTOR_TYPE_P (ltype))
> vect_get_load_cost (vinfo, stmt_info, 1,
> alignment_support_scheme,
> misalignment,
> false, &inside_cost, nullptr,
> cost_vec,
> cost_vec, true);
>
> as it's dr_unaligned_supported, it's costed as unaligned_load then causes the
> ICE. One idea is to teach rs6000_builtin_vectorization_cost about one single
> lane vector unaligned load as scalar_load. But I think it's simple to treat
> single lane vector load as scalar_load, as I expect veclower will lower it
> later.
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 4622d6a04ef..bdf4c12cd03 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -9985,7 +9985,9 @@ vectorizable_load (vec_info *vinfo,
> {
>if (costing_p)
> {
> - if (VECTOR_TYPE_P (ltype))
> + /* For a single lane vector type, we should cost it as
> + scalar_load to avoid ICE, see PR110776. */
> + if (VECTOR_TYPE_P (ltype) && lnel > 1)
> vect_get_load_cost (vinfo, stmt_info, 1,
> alignment_support_scheme,
> misalignment,
> false, &inside_cost, nullptr,
> cost_vec,
>
> Hi Richi, what do you think of this?
I think apart from the consideration what a single element vector
is compared to a scalar, a more to-the-point fix is
if (VECTOR_TYPE_P (ltype)
&& memory_access_type != VMAT_ELEMENTWISE)
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776
Kewen Lin changed:
What|Removed |Added
Target|powerpc-darwin |powerpc-darwin,
||powerpc64-linux
--- Comment #4 from Kewen Lin ---
When cooking a patch, I reduced the preprocessed file into:
int a;
long *b;
int c() {
long e;
int d = 0;
for (long f; f; f++) {
e = b[f * a];
if (e)
d = 1;
}
if (d)
for (;;)
;
}
and found the ICE can be reproduced on powerpc64 with -O2 -mcpu=power6
-maltivec.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776
Kewen Lin changed:
What|Removed |Added
CC||rguenth at gcc dot gnu.org
--- Comment #3 from Kewen Lin ---
The failure exposed one special case: there is one stmt
# VUSE <.MEM_404>
_15 = *_14;
its STMT_VINFO_STRIDED_P (stmt_info) is set, memory_access_type is
VMAT_ELEMENTWISE and alignment_support_scheme is dr_unaligned_supported, its
vector type is "vector(1) long int", so in the handling it's taken as:
/* Load vector(1) scalar_type if it's 1 element-wise vectype. */
else if (nloads == 1)
ltype = vectype;
as its ltype is vector type, we cost it with
if (VECTOR_TYPE_P (ltype))
vect_get_load_cost (vinfo, stmt_info, 1,
alignment_support_scheme, misalignment,
false, &inside_cost, nullptr, cost_vec,
cost_vec, true);
as it's dr_unaligned_supported, it's costed as unaligned_load then causes the
ICE. One idea is to teach rs6000_builtin_vectorization_cost about one single
lane vector unaligned load as scalar_load. But I think it's simple to treat
single lane vector load as scalar_load, as I expect veclower will lower it
later.
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 4622d6a04ef..bdf4c12cd03 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -9985,7 +9985,9 @@ vectorizable_load (vec_info *vinfo,
{
if (costing_p)
{
- if (VECTOR_TYPE_P (ltype))
+ /* For a single lane vector type, we should cost it as
+ scalar_load to avoid ICE, see PR110776. */
+ if (VECTOR_TYPE_P (ltype) && lnel > 1)
vect_get_load_cost (vinfo, stmt_info, 1,
alignment_support_scheme,
misalignment,
false, &inside_cost, nullptr,
cost_vec,
Hi Richi, what do you think of this?
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 Kewen Lin changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Assignee|unassigned at gcc dot gnu.org |linkw at gcc dot gnu.org Ever confirmed|0 |1 Last reconfirmed||2023-07-24 --- Comment #2 from Kewen Lin --- Thanks for reporting and sorry for the breakage. I'll have a look first. (In reply to Iain Sandoe from comment #0) > The ICE seems to be because rs6000_builtin_vectorization_cost () is called > with a request for a misaligned load (which we do not support), It > reproduces on a cross from x86_64. > > This is in compiling libgfortran generated code (so nothing Darwin-specific, > other than being an Altivec platform). Thanks for the information. > > A philosophical question; if a request is made for the cost of doing > something unsupported - should we not return "infinity" rather than ICEing? > > Presumably, the alternative is that the middle end needs to know that some > kinds of operation are not supported and therefore not to try and cost them > (speculation here; I have no knowledge of the relevant code). I think that's what's being adopted now, if the target doesn't support unaligned load, the middle-end should take it as dr_unaligned_unsupported (dr_alignment_support) and use VECT_MAX_COST, it's expected that there is no chance to query it with unaligned_load. Maybe some path was changed by the culprit commit.
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 Andrew Pinski changed: What|Removed |Added Keywords||build Target Milestone|--- |14.0
[Bug target/110776] [14 Regression] powerpc-darwin bootstrap broken after r14-2490 with ICE rs6000.cc:5069 building libgfortran
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110776 --- Comment #1 from Iain Sandoe --- Created attachment 55613 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55613&action=edit preprocessed (not reduced, but the function is not large)
