Bug 112759 - [13/14 regression] mips -march=native detection broken with gcc 13+ since r13-3178-g66c48be23e0fa5
Summary: [13/14 regression] mips -march=native detection broken with gcc 13+ since r13...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: driver (show other bugs)
Version: 13.1.0
: P3 normal
Target Milestone: 13.3
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-29 07:10 UTC by matoro
Modified: 2023-12-23 08:55 UTC (History)
4 users (show)

See Also:
Host: mips64-unknown-linux-gnu
Target: mips64-unknown-linux-gnu
Build: mips64-unknown-linux-gnu
Known to work: 12.3.1
Known to fail: 13.1.0
Last reconfirmed: 2023-12-13 00:00:00


Attachments
/proc/cpuinfo (619 bytes, text/plain)
2023-11-29 07:10 UTC, matoro
Details

Note You need to log in before you can comment on or make changes to this bug.
Description matoro 2023-11-29 07:10:14 UTC
Since gcc 13 - and presumably 66c48be23e0fa5ee7474b4b078e013f901c71eed since that is the only recent change to this area - detection of -march=native on mips is broken, defaulting to mips1.

With gcc 13:
# gcc -march=native -Q --help=target | grep "arch=" | head -n 1
  -march=ISA                            mips1
# gcc --version
gcc (Gentoo 13.2.1_p20231014 p9) 13.2.1 20231014

With gcc 12:
# gcc -march=native -Q --help=target | grep "arch=" | head -n 1
  -march=ISA                            octeon2
# gcc --version
gcc (Gentoo 12.3.1_p20230825 p2) 12.3.1 20230825

Contents of my /proc/cpuinfo are attached.  strace seems to indicate that it is reading the data at least:

openat(AT_FDCWD, "/proc/cpuinfo", O_RDONLY) = 3
statx(3, "", AT_STATX_SYNC_AS_STAT|AT_NO_AUTOMOUNT|AT_EMPTY_PATH, STATX_BASIC_STATS, {stx_mask=STATX_BASIC_STATS|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0444, stx_size=0, ...}) = 0
read(3, "system type\t\t: EBB6800 (CN6880p2"..., 1024) = 1024
close(3)                                = 0
Comment 1 matoro 2023-11-29 07:10:40 UTC
Created attachment 56714 [details]
/proc/cpuinfo
Comment 2 Andrew Pinski 2023-11-29 22:16:30 UTC
Hmm, looks like it is this part of the change:
+  if (cpu)
+    ret = reconcat (ret, ret, "-m", argv[0], "=", cpu, NULL);
 
-  return concat ("-m", argv[0], "=", cpu, NULL);


Maybe it should have been:
```
if (cpu)
  {
    if (!ret)
      ret = concat ("-m", argv[0], "=", cpu, NULL);
    else
      ret = reconcat (ret, ret, "-m", argv[0], "=", cpu, NULL);
  }
```

Can you try that?
Comment 3 matoro 2023-11-30 15:37:30 UTC
(In reply to Andrew Pinski from comment #2)
> Hmm, looks like it is this part of the change:
> +  if (cpu)
> +    ret = reconcat (ret, ret, "-m", argv[0], "=", cpu, NULL);
>  
> -  return concat ("-m", argv[0], "=", cpu, NULL);
> 
> 
> Maybe it should have been:
> ```
> if (cpu)
>   {
>     if (!ret)
>       ret = concat ("-m", argv[0], "=", cpu, NULL);
>     else
>       ret = reconcat (ret, ret, "-m", argv[0], "=", cpu, NULL);
>   }
> ```
> 
> Can you try that?

Indeed fixes it, thanks!
Comment 4 Andrew Pinski 2023-12-13 00:47:16 UTC
Confirmed.
Comment 5 YunQiang Su 2023-12-18 02:24:54 UTC
It's my fault.
I misunderstanding `reconcat`:

if `optr` is NULL, it cannot work as the `s1` at the sametime.
If so, the return string will be empty.

So, let's define and initial ret like this:

char *ret = concat(" "); 

Any idea?
Comment 6 YunQiang Su 2023-12-18 03:16:39 UTC
ohh, it should be concat(" ", NULL);
Comment 7 GCC Commits 2023-12-23 08:47:32 UTC
The master branch has been updated by YunQiang Su <syq@gcc.gnu.org>:

https://gcc.gnu.org/g:384dbb0b4e751e6eb7ba3f9993a6cce466743926

commit r14-6811-g384dbb0b4e751e6eb7ba3f9993a6cce466743926
Author: YunQiang Su <syq@gcc.gnu.org>
Date:   Tue Dec 19 07:36:52 2023 +0800

    MIPS: Put the ret to the end of args of reconcat [PR112759]
    
    The function `reconcat` cannot append string(s) to NULL,
    as the concat process will stop at the first NULL.
    
    Let's always put the `ret` to the end, as it may be NULL.
    We keep use reconcat here, due to that reconcat can make it
    easier if we add more hardware features detecting, for example
    by hwcap.
    
    gcc/
    
            PR target/112759
            * config/mips/driver-native.cc (host_detect_local_cpu):
            Put the ret to the end of args of reconcat.
Comment 8 GCC Commits 2023-12-23 08:51:15 UTC
The releases/gcc-13 branch has been updated by YunQiang Su <syq@gcc.gnu.org>:

https://gcc.gnu.org/g:63df799074351e9b3ab90f5b3031ba2691385af8

commit r13-8175-g63df799074351e9b3ab90f5b3031ba2691385af8
Author: YunQiang Su <syq@gcc.gnu.org>
Date:   Tue Dec 19 07:36:52 2023 +0800

    MIPS: Put the ret to the end of args of reconcat [PR112759]
    
    The function `reconcat` cannot append string(s) to NULL,
    as the concat process will stop at the first NULL.
    
    Let's always put the `ret` to the end, as it may be NULL.
    We keep use reconcat here, due to that reconcat can make it
    easier if we add more hardware features detecting, for example
    by hwcap.
    
    gcc/
    
            PR target/112759
            * config/mips/driver-native.cc (host_detect_local_cpu):
            Put the ret to the end of args of reconcat.
Comment 9 YunQiang Su 2023-12-23 08:55:40 UTC
It should be fixed now, and back ported to gcc13.

And we may should add some more detections:

1. hwcap
2. getauxval(AT_PLATFORM)
3. more cpuinfo parsing