Bug 29076 – Detect VFP at runtime in generic ARM port on Linux

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gabor Loki]

We should detect that the VFP is present at runtime instead of at build
time for ARM architectures. On Linux there is a possible solution to do
this. The /proc/self/auxv contains some architecture specific flags.
One of them is responsible for the VFP. I am going to attach a patch
which solves this issue.

[Gabor Loki]

Created attachment 39252 [details]
Detect VFP at runtime for arm-linux

[Mark Rowe (bdash)]

Comment on attachment 39252 [details]
Detect VFP at runtime for arm-linux

Does this code all need to live in the .h file?  It doesn't seem as though it needs to be inlined and given the fact that it's likely to become more #if'd in the future it would seem better placed in a .cpp file.

> diff --git a/JavaScriptCore/assembler/MacroAssemblerARM.h b/JavaScriptCore/assembler/MacroAssemblerARM.h
> index 1a4290d..36cee92 100644
> --- a/JavaScriptCore/assembler/MacroAssemblerARM.h
> +++ b/JavaScriptCore/assembler/MacroAssemblerARM.h
> @@ -35,10 +35,23 @@
>  #include "ARMAssembler.h"
>  #include "AbstractMacroAssembler.h"
>  
> +#if PLATFORM(LINUX)
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <elf.h>
> +#endif
> +
>  namespace JSC {
>  
>  class MacroAssemblerARM : public AbstractMacroAssembler<ARMAssembler> {
>  public:
> +    MacroAssemblerARM()
> +        : m_isVFPPresent(isVFPPresent())
> +    {
> +    }
> +
>      enum Condition {
>          Equal = ARMAssembler::EQ,
>          NotEqual = ARMAssembler::NE,
> @@ -637,8 +650,7 @@ public:
>      // Floating point operators
>      bool supportsFloatingPoint() const
>      {
> -        // FIXME: should be a dynamic test: VFP, FPA, or nothing
> -        return false;
> +        return m_isVFPPresent;
>      }
>  
>      bool supportsFloatingPointTruncate() const
> @@ -793,6 +805,29 @@ private:
>          ARMAssembler::relinkCall(call.dataLocation(), destination.executableAddress());
>      }
>  
> +    bool isVFPPresent()
> +    {
> +#if PLATFORM(LINUX)
> +        bool has_VFP = false;
> +        int fd = open("/proc/self/auxv", O_RDONLY);
> +        if (fd > 0) {
> +            Elf32_auxv_t aux;
> +            while(read(fd, &aux, sizeof(Elf32_auxv_t))) {
> +                if (aux.a_type == AT_HWCAP) {
> +                    has_VFP = (aux.a_un.a_val & 64) != 0;
> +                    break;
> +                }
> +            }
> +            close(fd);
> +        }
> +        return has_VFP;


This will open /proc/self/auxv and perform this check again each time a MacroAssemblerARM instance is created.  Since the presence of VFP isn't something that will change during the lifetime of a process it seems that we should calculate this just the once.

> +#else
> +        /* Disable by default.  */
> +        return false;
> +#endif
> +    }
> +
> +    const bool m_isVFPPresent;

It seems a bit odd to store this in a member variable given that the value cannot differ between instances.

[Gabor Loki]

(In reply to comment #2)
The MacroAssemblerX86 works in the same way.

Unfortunately, we cannot provide an ARM instruction specific solution to
detect VFP without getting illegal instruction error (as x86 cpuid). So
we have to use a kernel specific support for that.

[Gabor Loki]

Created attachment 39261 [details]
Detect VFP at runtime for arm-linux (v2)

I changed the patch according to bdash and gbarra suggestions.
- Add MacroAssemblerARM.cpp which contains the VFP check.
- Declare a static const variable to hold the results of isVFPPresent function.

[Gavin Barraclough]

Comment on attachment 39261 [details]
Detect VFP at runtime for arm-linux (v2)

A few minor tweaks.

isVFPPresent() should be marked static since it is not used outside this file.

> return (aux.a_un.a_val & HWCAP_VFP);

We try to skip unnecessary parentheses, these should be removed.

The #if should probably also be !PLATFORM_ARM_ARCH(7)

[Gabor Loki]

Created attachment 39326 [details]
Detect VFP at runtime for arm-linux (v3)

[Zoltan Horvath]

Landed in @48389.  http://trac.webkit.org/changeset/48389