Skip to content
Permalink
Browse files
Properly set numFPRs on ARM with NEON/VFP_V3_D32
https://bugs.webkit.org/show_bug.cgi?id=227212

Reviewed by Filip Pizlo.

Don't hardcode the number of FP regs on ARMv7 to 16; when targetting a
CPU with NEON or VFP_V3_d32, the number of FP regs is 32.

This also reverts the recent change to add an extra word to RegisterSet
which essentially covered up for this mismatch. The reason this bug only
manifested on certain compiler versions was that GCC 8.4/8.5 where built using
our buildroot infrastructure, whereas the other GCC versions we tested with
were debian system toolchains, targetting a lowest common denominator.

* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::std::initializer_list<int>):
* jit/RegisterSet.h:


Canonical link: https://commits.webkit.org/239024@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279107 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
aoikonomopoulos committed Jun 22, 2021
1 parent 5ec749f commit 67d0f30
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 21 deletions.
@@ -1,3 +1,23 @@
2021-06-22 Angelos Oikonomopoulos <angelos@igalia.com>

Properly set numFPRs on ARM with NEON/VFP_V3_D32
https://bugs.webkit.org/show_bug.cgi?id=227212

Reviewed by Filip Pizlo.

Don't hardcode the number of FP regs on ARMv7 to 16; when targetting a
CPU with NEON or VFP_V3_d32, the number of FP regs is 32.

This also reverts the recent change to add an extra word to RegisterSet
which essentially covered up for this mismatch. The reason this bug only
manifested on certain compiler versions was that GCC 8.4/8.5 where built using
our buildroot infrastructure, whereas the other GCC versions we tested with
were debian system toolchains, targetting a lowest common denominator.

* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::std::initializer_list<int>):
* jit/RegisterSet.h:

2021-06-21 Ross Kirsling <ross.kirsling@sony.com>

[JSC] Add JIT ICs for `#x in obj` feature
@@ -28,6 +28,8 @@

#if ENABLE(ASSEMBLER)

#include <initializer_list>

#include "ARMv7Assembler.h"
#include "AbstractMacroAssembler.h"

@@ -43,9 +45,10 @@ class MacroAssemblerARMv7 : public AbstractMacroAssembler<Assembler> {
inline ARMRegisters::FPSingleRegisterID fpTempRegisterAsSingle() { return ARMRegisters::asSingle(fpTempRegister); }

public:
static constexpr unsigned numGPRs = 16;
static constexpr unsigned numFPRs = 16;

#define DUMMY_REGISTER_VALUE(id, name, r, cs) 0,
static constexpr unsigned numGPRs = std::initializer_list<int>({ FOR_EACH_GP_REGISTER(DUMMY_REGISTER_VALUE) }).size();
static constexpr unsigned numFPRs = std::initializer_list<int>({ FOR_EACH_FP_REGISTER(DUMMY_REGISTER_VALUE) }).size();
#undef DUMMY_REGISTER_VALUE
RegisterID scratchRegister() { return dataTempRegister; }

MacroAssemblerARMv7()
@@ -32,27 +32,10 @@
#include "Reg.h"
#include "TempRegisterSet.h"
#include <wtf/Bitmap.h>
#include <wtf/Compiler.h>

namespace JSC {

#if CPU(ARM) && COMPILER(GCC)

#if GCC_VERSION_AT_LEAST(8, 4, 0) && !GCC_VERSION_AT_LEAST(9, 0, 0)
// GCC 8.4.0 and 8.5.0 seem to miscompile WTF:Bitmap::count code on
// ARM, something that apparently was covered up by the extra
// word. The issue seems to not manifest with GCC 8.3.0 and >9.
// Temporarily cover up by adding back the + 1.
#define REGISTERSET_BITMAP_SLACK 1
#else
#define REGISTERSET_BITMAP_SLACK 0
#endif

#else
#define REGISTERSET_BITMAP_SLACK 0
#endif

typedef Bitmap<MacroAssembler::numGPRs + MacroAssembler::numFPRs + REGISTERSET_BITMAP_SLACK> RegisterBitmap;
typedef Bitmap<MacroAssembler::numGPRs + MacroAssembler::numFPRs> RegisterBitmap;
class RegisterAtOffsetList;

class RegisterSet {

0 comments on commit 67d0f30

Please sign in to comment.