Richard,
I tried to untangle it and made the following naming changes: @2 == op2/op2_mode @3 == op3/op3_mode @4 ==sel4 ... It is in patch-v3 which I'm about to submit. Let me know if you think it's sane scheme. Thanks, Igor ________________________________ From: Richard Biener <[email protected]> Sent: Wednesday, October 14, 2026 7:23 AM To: Igor Shevlyakov Cc: [email protected] Subject: Re: [PATCH v2 1/2] match.pd: Fix mode queries of VEC_PERM merge through view-convert [PR127882] [EXTERNAL] Caution: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. On Tue, 13 Oct 2026, Igor Shevlyakov wrote: > The pattern that merges c = VEC_PERM_EXPR <a, b> into > d = VEC_PERM_EXPR <c, c> also matches when c is view-converted before d > uses it. It asks can_vec_perm_const_p about the new permute and about > both original ones with d's mode as the result mode and a's mode as the > input mode. When the conversion changes the element type, e.g. from > short to _Float16, that is a permute between two different modes, which > the pattern never emits and no target supports. All three queries then > fail, "one of the original permutes can't be handled" holds, and the > permutes are merged even when both are supported and the merged one is > not. Vector lowering then expands the result element by element: on > x86_64 with SSE2 the testcase goes from 3 to 13 instructions. > > Query each permute with its own modes instead: the new one has a's > type (it is view-converted to d's type afterwards), the inner one > produces c from a and b, and the outer one produces d from the > converted c. > > Assisted-by: Claude Opus 5.5 > > PR middle-end/127882 > > gcc/ChangeLog: > > * match.pd (vec_perm (view_convert? (vec_perm)) merge): Query > each permute with its own modes. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/pr127882.c: New test. > > Signed-off-by: Igor Shevlyakov <[email protected]> > --- > gcc/match.pd | 13 ++++++++---- > gcc/testsuite/gcc.target/i386/pr127882.c | 27 ++++++++++++++++++++++++ > 2 files changed, 36 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr127882.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 6984af44582..fcb67a8dcc0 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -12875,7 +12875,11 @@ and, > (if (TYPE_VECTOR_SUBPARTS (type).is_constant ()) > (with > { > + /* c may be view-converted before d uses it, so query each permute > + with its own modes. The new permute has the type of a. */ > machine_mode result_mode = TYPE_MODE (type); > + machine_mode c_op_mode = TYPE_MODE (TREE_TYPE (@0)); > + machine_mode c_mode = TYPE_MODE (TREE_TYPE (@1)); > machine_mode op_mode = TYPE_MODE (TREE_TYPE (@2)); I think the naming is now confusing, result_mode would be d_mode, op_mode is ab_mode and since the comment does not show the VIEW_CONVERT c and c_op mode do not match anything. I would suggest to use op{0,1,2}_mode to match the capture IDs and to adjust the comment to mention the VIEW_CONVERT and replace a,b,c,d with op{0,1,2} and result. OK with that change or whatever similar change to make this more obvious. Richard. > int nelts = TYPE_VECTOR_SUBPARTS (type).to_constant (); > vec_perm_indices sel0, sel1; > @@ -12898,11 +12902,12 @@ and, > If one or both of the original VEC_PERM_EXPRs can't be > handled and the new one can't be either, don't increase > number of VEC_PERM_EXPRs that can't be handled. */ > - if (can_vec_perm_const_p (result_mode, op_mode, sel2, false) > + if (can_vec_perm_const_p (op_mode, op_mode, sel2, false) > || (single_use (@0) > - ? (!can_vec_perm_const_p (result_mode, op_mode, sel0, true) > - || !can_vec_perm_const_p (result_mode, op_mode, sel1, true)) > - : !can_vec_perm_const_p (result_mode, op_mode, sel1, true))) > + ? (!can_vec_perm_const_p (c_mode, op_mode, sel0, true) > + || !can_vec_perm_const_p (result_mode, c_op_mode, sel1, > + true)) > + : !can_vec_perm_const_p (result_mode, c_op_mode, sel1, true))) > op0 = vec_perm_indices_to_tree (TREE_TYPE (@5), sel2); > } > (if (op0) > diff --git a/gcc/testsuite/gcc.target/i386/pr127882.c > b/gcc/testsuite/gcc.target/i386/pr127882.c > new file mode 100644 > index 00000000000..531a30b3c45 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr127882.c > @@ -0,0 +1,27 @@ > +/* PR middle-end/127882 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -msse2 -mno-ssse3 -fdump-tree-optimized" } */ > + > +typedef short v8hi __attribute__((vector_size(16))); > +typedef _Float16 v8hf __attribute__((vector_size(16))); > + > +/* Both permutes are supported, but the merged one is not: keep them, also > + with a VIEW_CONVERT_EXPR between them. */ > + > +v8hi > +f1 (v8hi a, v8hi b) > +{ > + v8hi c = __builtin_shufflevector (a, b, 8, 0, 9, 1, 10, 2, 11, 3); > + return __builtin_shufflevector (c, c, 1, 1, 0, 0, 4, 5, 7, 7); > +} > + > +v8hf > +f2 (v8hi a, v8hi b) > +{ > + v8hi c = __builtin_shufflevector (a, b, 8, 0, 9, 1, 10, 2, 11, 3); > + v8hf e = (v8hf) c; > + return __builtin_shufflevector (e, e, 1, 1, 0, 0, 4, 5, 7, 7); > +} > + > +/* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 4 "optimized" } } */ > +/* { dg-final { scan-tree-dump-not "BIT_FIELD_REF" "optimized" } } */ > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
