Bug 107717 - [13 Regression] ICEs expanding permutes after g:dc95e1e9702f2f6367bbc108c8d01169be1b66d2
Summary: [13 Regression] ICEs expanding permutes after g:dc95e1e9702f2f6367bbc108c8d01...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: 13.0
Assignee: Tamar Christina
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2022-11-16 15:13 UTC by Tamar Christina
Modified: 2022-11-18 08:24 UTC (History)
1 user (show)

See Also:
Host:
Target: aarch64*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tamar Christina 2022-11-16 15:13:29 UTC
After

commit dc95e1e9702f2f6367bbc108c8d01169be1b66d2 (origin/trunk, origin/master, origin/HEAD)
Author: Hongyu Wang <hongyu.wang@intel.com>
Date:   Mon Jan 17 13:01:51 2022 +0800

    Optimize VEC_PERM_EXPR with same permutation index and operation

    The sequence
         c1 = VEC_PERM_EXPR (a, a, mask)
         c2 = VEC_PERM_EXPR (b, b, mask)
         c3 = c1 op c2
    can be optimized to
         c = a op b
         c3 = VEC_PERM_EXPR (c, c, mask)
    for all integer vector operation, and float operation with
    full permutation.

    gcc/ChangeLog:

            PR target/98167
            * match.pd: New perm + vector op patterns for int and fp vector.

    gcc/testsuite/ChangeLog:

            PR target/98167
            * gcc.target/i386/pr98167.c: New test.

We see various ICEs, an example is

void foo(int n, char *restrict out, char *restrict in) {
  for (int i=n; i-->0; ) {
    out[i] += in[i];
  }
}

compiled with

aarch64-none-linux-gnu -O3 -march=armv8-a+sve2

The problem is that the match.pd pattern as written causes the permute to switch from a single register permute to a two register one.

The reason is that when the folded result is expanded in SSA form

vec_perm (op @0 @1) (op @0 @1)

the result of applying op twice results in two distinct SSA names. This fails because expand_vec_perm_const now tries to use a two operand expansion because there's no easy way to tell that these two operands are the same.

If it happens early enough we can CSE the operands, but when this happens after vec_lower it generated something the target does not support.

I tried getting expand_vec_perm_const to recognize that they are the same, but that's quite hard.

It's best to prevent the generation of the two SSA names to begin with, or add an additional rule for match.pd that's able to CSE this.

I'm making this issue because I don't know which approach upstream would like so it's easier to ask first.
Comment 1 Tamar Christina 2022-11-16 15:52:11 UTC
testing a patch.
Comment 2 GCC Commits 2022-11-17 08:22:10 UTC
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:cbe313060cdcf1d857d42a9e16a1a03e5ff89fff

commit r13-4123-gcbe313060cdcf1d857d42a9e16a1a03e5ff89fff
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Nov 17 08:20:59 2022 +0000

    middle-end: ensure that VEC_PERM operands get lowered to the same SSA_NAME. [PR107717]
    
    At the moment when the VEC_PERMs generated by this match.pd rule is generated
    it creates two different SSA_NAMEs for the folded operand.  Because of this it
    the permute switches from a single operand permute to a two operand permute and
    the target may no longer support a permute for this.
    
    This fixes it by ensuring we generate the same SSA_NAME for both operands.
    
    gcc/ChangeLog:
    
            PR tree-optimization/107717
            * match.pd: Ensure same SSA_NAME.
    
    gcc/testsuite/ChangeLog:
    
            PR tree-optimization/107717
            * gcc.target/aarch64/sve2/pr107717.c: New test.
Comment 3 Tamar Christina 2022-11-17 08:29:21 UTC
Fixed
Comment 4 Hongyu Wang 2022-11-18 08:24:24 UTC
(In reply to Tamar Christina from comment #3)
> Fixed

Thanks for the fix! It also give me a good tip for match pattern writing :)