[Bug target/96366] [AArch64] ICE due to lack of support for VNx2SI sub instruction

[bule1 at huawei dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96366

--- Comment #2 from Bu Le <bule1 at huawei dot com> ---
(In reply to rsandifo@gcc.gnu.org from comment #1)
> (In reply to Bu Le from comment #0)
> Hmm.  In general, the lack of a vector pattern shouldn't case ICEs,
> but I suppose the add/sub pairing is somewhat special because of
> the canonicalisation rules.  It would be worth looking at exactly
> why we generate the subtract though, just to confirm that this is
> an “expected” ICE rather than a symptom of a deeper problem.

Sure. The logic is that the subtraction will be expanded in expr.c:8989, before
which I believe it still works fine. The gimple to be expand is 

vect__5.16_77 = { 4294967273, 4294967154, 4294967294, 4294967265 } -
vect__1.14_73

When the logic goes on, it went into the binop routine at expr:9948 because op1
(vect__1.14_73) is not a const op and missed the oppotunity to turn into an add
negative pair. Then,the routine will call expand_binop to finalize the
subtraction. The expand_binop function also has an oppotunity to turn this
subtraction to add a negative number, but also missed because op1 is not a
constant.

It occurs to me that we can brought the check for the availbility of this
pattern to the decision condition for whether turning the subtraction to a
addition of negative equivalent. This can be an insurance measurement for the
similar case that the pattern is missed, preventing the ICE. So I tried
following change, which turns out could also solve the problem by turning the
subtraction into addition as expected.

diff -Nurp gcc-20200728-org/gcc/optabs.c gcc-20200728/gcc/optabs.c
--- gcc-20200728-org/gcc/optabs.c       2020-07-29 15:53:52.760000000 +0800
+++ gcc-20200728/gcc/optabs.c   2020-07-30 11:00:00.964000000 +0800
@@ -1171,10 +1171,12 @@ expand_binop (machine_mode mode, optab b

   mclass = GET_MODE_CLASS (mode);

-  /* If subtracting an integer constant, convert this into an addition of
-     the negated constant.  */
+  /* If subtracting an integer constant, or if no subtraction pattern
available
+     for this mode, convert this into an addition of the negated constant.  */

-  if (binoptab == sub_optab && CONST_INT_P (op1))
+  if (binoptab == sub_optab
+      && (CONST_INT_P (op1)
+      || optab_handler (binoptab, mode) == CODE_FOR_nothing))
     {
       op1 = negate_rtx (mode, op1);
       binoptab = add_optab;


> The idea was for that patch to add the bare minimum needed
> to support the “unpacked vector” infrastructure.  Then, once the
> infrastructure was in place, we could add support for other
> unpacked vector operations too.
> 
> However, the infrastructure went in late during the GCC 10
> cycle, so the idea was to postpone any new unpacked vector
> support to GCC 11.  So far the only additional operations
> has been Joe Ramsay's patches for logical operations
> (g:bb3ab62a8b4a108f01ea2eddfe31e9f733bd9cb6 and
> g:6802b5ba8234427598abfd9f0163eb5e7c0d6aa8).
> 
> The reason for not changing many operations at once is that,
> in each case, a decision needs to be made whether the
> operation should use the container mode (as for INDEX),
> the element mode (as for right shifts, once they're
> implemented) or whether it doesn't matter (as for addition).
> Each operation also needs tests.  So from that point of view,
> it's more convenient to have a separate patch for each
> operation (or at least closely-related groups of operations).

Oh, I see. From the performance's point of view, I beleive that add the
subtraction pattern is necessary eventually. I compiled and ran the test case
attached with the subtraction pattern sulotion, which works fine. Logically,
the subtraction should be the same as the addition, which is not sensetive to
the operation mode.

My idea of solving this problem is that we upstream the patch for mode
extension independently, after which upstream the sub-to-add patch for
insurance that other cases might step into the same routine.

Any suggestions?