[Bug libfortran/110966] New: should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[Bug libfortran/110966] New: should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[crazylht at gmail dot com]

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

            Bug ID: 110966
           Summary: should matmul_c8_avx512f be updated with
                    matmul_c8_x86-64-v4.
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libfortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: crazylht at gmail dot com
  Target Milestone: ---

In libgfortran/m4/matmul.m4, we have

#ifdef HAVE_AVX512F
'define(`matmul_name',`matmul_'rtype_code`_avx512f')dnl
`static void
'matmul_name` ('rtype` * const restrict retarray,
        'rtype` * const restrict a, 'rtype` * const restrict b, int try_blas,
        int blas_limit, blas_call gemm) __attribute__((__target__("avx512f")));
static' include(matmul_internal.m4)dnl
`#endif  /* HAVE_AVX512F */


But target ("avx512f") only enable -mavx512f which has quite limited capability
of AVX512. Since now we have arch level, should we use target("arch=x86-64-v4")
instead.

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-*

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think matmul is fine with avx512f or avx, so requiring/using only the base
ISA level sounds fine to me.

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[crazylht at gmail dot com]

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

--- Comment #2 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Richard Biener from comment #1)
> I think matmul is fine with avx512f or avx, so requiring/using only the base
> ISA level sounds fine to me.

Could be potential miss-optimization.

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[anlauf at gcc dot gnu.org]

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

--- Comment #3 from anlauf at gcc dot gnu.org ---
(In reply to Hongtao.liu from comment #2)
> (In reply to Richard Biener from comment #1)
> > I think matmul is fine with avx512f or avx, so requiring/using only the base
> > ISA level sounds fine to me.
> 
> Could be potential miss-optimization.

Do you mean a missed optimzation?

Or really wrong code?

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[crazylht at gmail dot com]

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

--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to anlauf from comment #3)
> (In reply to Hongtao.liu from comment #2)
> > (In reply to Richard Biener from comment #1)
> > > I think matmul is fine with avx512f or avx, so requiring/using only the base
> > > ISA level sounds fine to me.
> > 
> > Could be potential miss-optimization.
> 
> Do you mean a missed optimzation?
> 
> Or really wrong code?

a missed optimzation.

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[tkoenig at gcc dot gnu.org]

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

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |tkoenig at gcc dot gnu.org
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2023-11-13

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Hongtao.liu from comment #4)
> (In reply to anlauf from comment #3)
> > (In reply to Hongtao.liu from comment #2)
> > > (In reply to Richard Biener from comment #1)
> > > > I think matmul is fine with avx512f or avx, so requiring/using only the base
> > > > ISA level sounds fine to me.
> > > 
> > > Could be potential miss-optimization.
> > 
> > Do you mean a missed optimzation?
> > 
> > Or really wrong code?
> 
> a missed optimzation.

Are there benchmarks which show that the code would indeed run
faster?

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[crazylht at gmail dot com]

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

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Thomas Koenig from comment #5)
> (In reply to Hongtao.liu from comment #4)
> > (In reply to anlauf from comment #3)
> > > (In reply to Hongtao.liu from comment #2)
> > > > (In reply to Richard Biener from comment #1)
> > > > > I think matmul is fine with avx512f or avx, so requiring/using only the base
> > > > > ISA level sounds fine to me.
> > > > 
> > > > Could be potential miss-optimization.
> > > 
> > > Do you mean a missed optimzation?
> > > 
> > > Or really wrong code?
> > 
> > a missed optimzation.
> 
> Are there benchmarks which show that the code would indeed run
> faster?

Not yet, just better in theory .
But considering that there might be some tweaks regarding x86-64-v4, I think
it's best to leave it unchanged for the time being.

[Bug libfortran/110966] should matmul_c8_avx512f be updated with matmul_c8_x86-64-v4.

[liuhongt at gcc dot gnu.org]

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

Hongtao Liu <liuhongt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|WAITING                     |RESOLVED
                 CC|                            |liuhongt at gcc dot gnu.org