Bug 490520 - Unsupported instruction in amd test
Summary: Unsupported instruction in amd test
Status: RESOLVED WAITINGFORINFO
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.24 GIT
Platform: unspecified Linux
: NOR crash
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-07-19 19:01 UTC by Vedant Paranjape
Modified: 2024-07-22 16:46 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vedant Paranjape 2024-07-19 19:01:07 UTC
SUMMARY
The testcase under the amd directory uses asm directives with an instruction that is not supported

testcase: https://sourceware.org/git/?p=valgrind.git;a=blob;f=none/tests/amd64/loopnel.c;h=88bc0996982319b580f30471d076a90986743acf;hb=HEAD

loopnel is not a supported instruction by amd x86 as per the latest amd document: https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24594.pdf

instead it should be changed to loopne

This leads to a crash

SOFTWARE/OS VERSIONS
Linux/KDE Plasma:
Comment 1 Paul Floyd 2024-07-20 06:37:11 UTC
The testcase has been there for 13 years. There is a configure test for loopnel and if the assembler does not support it the testcase does not get built.

Does that mean that AMD are now making CPUs that have removed this opcode?

GCC supports it https://godbolt.org/z/5Wrqrqev1 and it works fine on my old Xeon PC.
LLVM doesn't support it

paulf> clang -g -o loopnel loopnel.c
loopnel.c:8:17: error: invalid instruction mnemonic 'loopnel'
  asm volatile ("1: addq $1, %0; loopnel 1b" : "+a" (rax), "+c" (rcx) : : "cc");
                ^
<inline asm>:1:20: note: instantiated into assembly here
        1: addq $1, %rax; loopnel 1b

I ought to look at that one day.

Finally I just tested on a machine that is
  Model name:             AMD EPYC 7773X 64-Core Processor

and it works OK.

Other than AMD not documenting it (and LLVM not accepting the mnemonic) I don't see any problem.
Comment 2 Vedant Paranjape 2024-07-22 11:51:37 UTC
> Other than AMD not documenting it (and LLVM not accepting the mnemonic) I don't see any problem.

It leads to regression in testcases for LLVM based compilers as LLVM doesn't accept the mnemonic. Do you suggest that I raise this with LLVM to add support for this instruction ?

I believe it's best to drop it from the testcase or change it to loopne as "officially" AMD has not documented it anyways.
Comment 3 Paul Floyd 2024-07-22 13:49:05 UTC
I've been using LLVM compilers for several years without any issues.

Are you seeing configure passing this test:

# does the x86/amd64 assembler understand the LOOPNEL instruction?
# Note, this doesn't generate a C-level symbol.  It generates a
# automake-level symbol (BUILD_LOOPNEL_TESTS), used in test Makefile.am's
AC_MSG_CHECKING([if x86/amd64 assembler supports 'loopnel'])

AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[
  do {
      __asm__ __volatile__("1:  loopnel 1b\n");
  } while (0)
]])], [
  ac_have_as_loopnel=yes
  AC_MSG_RESULT([yes])
], [
  ac_have_as_loopnel=no
  AC_MSG_RESULT([no])
])

AM_CONDITIONAL([BUILD_LOOPNEL_TESTS], [test x$ac_have_as_loopnel = xyes])

and hence none/tests/amd64/Makefile gets a rule to build the loopnel test, but then "make check" fails to build the loopnel testcase?
Comment 4 Vedant Paranjape 2024-07-22 16:46:43 UTC
(In reply to Paul Floyd from comment #3)
> I've been using LLVM compilers for several years without any issues.
> 
> Are you seeing configure passing this test:
> 
> # does the x86/amd64 assembler understand the LOOPNEL instruction?
> # Note, this doesn't generate a C-level symbol.  It generates a
> # automake-level symbol (BUILD_LOOPNEL_TESTS), used in test Makefile.am's
> AC_MSG_CHECKING([if x86/amd64 assembler supports 'loopnel'])
> 
> AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[
>   do {
>       __asm__ __volatile__("1:  loopnel 1b\n");
>   } while (0)
> ]])], [
>   ac_have_as_loopnel=yes
>   AC_MSG_RESULT([yes])
> ], [
>   ac_have_as_loopnel=no
>   AC_MSG_RESULT([no])
> ])
> 
> AM_CONDITIONAL([BUILD_LOOPNEL_TESTS], [test x$ac_have_as_loopnel = xyes])
> 
> and hence none/tests/amd64/Makefile gets a rule to build the loopnel test,
> but then "make check" fails to build the loopnel testcase?

Yes you are right, that should not have gotten through. Let me check again, maybe I am missing something.