[Bug target/116358] New: undefined reference to `__umindi3' at -O3 when compiling with SVE

[Bug target/116358] New: undefined reference to `__umindi3' at -O3 when compiling with SVE

[ktkachov at gcc dot gnu.org]

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

            Bug ID: 116358
           Summary: undefined reference to `__umindi3' at -O3 when
                    compiling with SVE
           Product: gcc
           Version: 15.0
            Status: UNCONFIRMED
          Keywords: aarch64-sve, link-failure
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ktkachov at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64

The following code:
#include <algorithm>
char a;
int b;
int var_20;
char var_43;
void c(int d, int e, short f, long long g[][5], int h[][5]) {
  long long i;
  for (bool m; m < a; m = b)
    if ((0100 ? ~e : 0) ? 0 : std::min((long long)f, i))
      for (bool j = 0; d ?: m ?: 0;)
        for (int k; k < 80485270; k += 4) {
          for (char l; l < 4; l += 3)
            var_20 = 0;
          for (long n(std::min((unsigned long long)(h[k][m] ? d : 0),
                               e ? 3 ? (unsigned long long)16 : 0 : 0) -
                      16);
               n < 4; n++)
            var_43 = d ? std::min(g[m][j], (long long)d) : 0;
        }
}

int main() {return 0;}

Compiled and linked with -O3 -march=armv9-a breaks at link-time:
(.text+0x15c): undefined reference to `__umindi3'
Interestingly, the presence of SVE seems to be the triggering factor.
-O3 -march=armv9-a+nosve links successfully

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=116353
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |15.0
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-08-13

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am 99% sure this was introduced by r15-2890-g72c9b5f438f22c .

umindi3 is the libcall for unsigned min for DI mode.
And it gets formed when ifcvt of the following BB:
(note 92 91 93 15 [bb 15] NOTE_INSN_BASIC_BLOCK)
(insn 93 92 96 15 (set (reg:DI 205 [ _119 ])
        (sign_extend:DI (reg:SI 345)))
"/opt/compiler-explorer/arm64/gcc-trunk-20240813/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/include/c++/15.0.0/bits/stl_algobase.h":238:7
105 {*extendsidi2_aarch64}
     (nil))
(note 96 93 97 15 NOTE_INSN_DELETED)
(note 97 96 98 15 NOTE_INSN_DELETED)
(note 98 97 99 15 NOTE_INSN_DELETED)
(insn 99 98 100 15 (parallel [
            (set (reg:DI 146 [ _123 ])
                (plus:DI (umin:DI (reg:DI 205 [ _119 ])
                        (const_int 16 [0x10]))
                    (const_int -16 [0xfffffffffffffff0])))
            (clobber (reg:CC 66 cc))
        ]) "/app/example.cpp":15:72 discrim 4 500 {*aarch64_minmax_plus}
     (expr_list:REG_UNUSED (reg:CC 66 cc)
        (expr_list:REG_DEAD (reg:DI 205 [ _119 ])
            (nil))))
;;  succ:       16 [always]  count:52559662 (estimated locally, freq 45.9480)
(FALLTHRU)
;; lr  out       29 [x29] 31 [sp] 64 [sfp] 65 [ap] 106 114 117 120 125 130 132
133 146 189 190 191 192 193 285 288 289 324 327 328 329 330 332 336 338
;; live  out     29 [x29] 31 [sp] 64 [sfp] 65 [ap] 106 114 117 120 125 130 132
133 146 189 190 191 192 193 285 288 289 324 327 328 329 330 332 336 338

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE

[tsamismanolis at gmail dot com]

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

--- Comment #2 from Manolis Tsamis <tsamismanolis at gmail dot com> ---
This is indeed due to r15-2890-g72c9b5f438f22c, but it is a different issue
from PR116353 (the fix for PR116353 doesn't solve this issue).

For some additional context, the basic block from the testcase that causes the
issue looks like this:

if (r204 == 0) {
  r205 = (sign_extend:DI) r345
  r146 = (plus:DI)
          ((umin:DI) r205 (const_int 16))
          (const_int -16)
}

And when the rhs for r146 is given to ifcvt's try_emit_cmove_seq, we get back
the __umindi3 call, and the results is:

r205 = (sign_extend:DI) r345
x0 = r205
x1 = (const_int 16)
x0 = call __umindi3 ...
r357 = x0
r355 = (plus:DI) (r357) (const_int -16)
cc = (r204 == 0)
r146 = (if_then_else:DI) (ne (cc) (const_int 0)) (r355) (r146)

Now, I don't know why emit_conditional_move returns the __umindi3 call (maybe
something to do with aarch64's md file?), but I believe we should reject a
sequence with a call in noce_convert_multiple_sets anyway.

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Reduced testcase:
```
unsigned long long *ttt;
long long f(int b, int c, unsigned long long d, int *tt)
{
  unsigned long long t3 = 0;
  int i = 0;
  unsigned long long t = d;
  if (!tt[0]) ;
  else {
    long long bb = b;
    unsigned long long a = bb;
    unsigned long long t2 = (a < 16 ? a : 16);
    t =  t2 - 16;
  }
  tt[0] = t;
  return 0;
}
```

Just need -O2.

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Slightly cleaned up reduced testcase:
```
long long f(int b, int c, long long d)
{
  if (c) {
    long long bb = b;
    long long t2 = (bb < 16 ? bb : 16);
    d =  t2 - 16;
  }
  return d;
}
```

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE

[ptomsich at gcc dot gnu.org]

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

ptomsich at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ptomsich at gcc dot gnu.org

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE since r15-2890-g72c9b5f438f22c

[tsamismanolis at gmail dot com]

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

--- Comment #5 from Manolis Tsamis <tsamismanolis at gmail dot com> ---
I had some trouble figuring the preferred way to address this. I made a choice
by answering these questions:

1) Should the aarch64 min/max expand pattern be improved?

This comment hints that we should get a cmp+csel when expanding this rather
than a libcall:

;; Expander for integer smin, smax, umin.  Mainly used to generate
;; straightforward RTL for TARGET_CSSC.  When that is not available
;; FAIL and let the generic expanders generate the CMP + CSEL sequences,
;; except for the SMIN and SMAX with zero cases, for which we have a
;; single instruction even for the base architecture.

Although aarch64.md could be changed to fix this, this wouldn't prevent similar
issues from happening in other cases.

2) Should force_operand handle this in a better way?

In theory, the expression involved can be forced in a register, but it's not
due to implementation details of force_operand. But I also believe changing
force_operand is too risky and out of scope.

3) Should noce_can_force_operand reject this RTL?

noce_can_force_operand is already based on internals of force_operand and it
would make sense to fix it there, but it doesn't look possible. As far as
force_operand is conserned there's no error, just a libcall being emitted.
Hence we would have to hardcode cases in noce_can_force_operand which is what
r15-2890-g72c9b5f438f22c tried to avoid compared to the previous
implementation.

4) Should we reject calls being generated in noce_convert_multiple_sets?

Although this initially felt like a workaround, I believe is the best solution
here. The reasoning is that, regardless of other factors,
noce_convert_multiple_sets should never emit a call expression. Since this was
already done for jump instructions, the change is also minimal:

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index da59c907891..b136d7dbbba 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3550,7 +3550,7 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
     return false;

   for (insn = seq; insn; insn = NEXT_INSN (insn))
-    if (JUMP_P (insn)
+    if (JUMP_P (insn) || CALL_P (insn)
        || recog_memoized (insn) == -1)
       return false;
-- 

Sent to the lists:
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/661145.html

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE since r15-2890-g72c9b5f438f22c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Philipp Tomsich <ptomsich@gcc.gnu.org>:

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

commit r15-3137-gc9e2d0ec6eabc2a6b8c00984b2b2bc48565bb99b
Author: Manolis Tsamis <manolis.tsamis@vrull.eu>
Date:   Thu Aug 22 02:59:11 2024 -0700

    ifcvt: disallow call instructions in noce_convert_multiple_sets [PR116358]

    Similar to not allowing jump instructions in the generated code, we
    also shouldn't allow call instructions in noce_convert_multiple_sets.
    In the case of PR116358 a libcall was generated from force_operand.

            PR middle-end/116358

    gcc/ChangeLog:

            * ifcvt.cc (noce_convert_multiple_sets): Disallow call insns.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/pr116358.c: New test.

[Bug middle-end/116358] [15 Regression] undefined reference to `__umindi3' at -O3 when compiling with SVE since r15-2890-g72c9b5f438f22c

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.