Bug 235112 – AssemblyHelpersSpoolers: use load/store pair on ARMv7

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Angelos Oikonomopoulos]

AssemblyHelpersSpoolers: use load/store pair on ARMv7

[Angelos Oikonomopoulos]

Created attachment 448939 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 449044 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 449054 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 449322 [details]
Patch

[Geza Lore]

Comment on attachment 449322 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449322&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:922
> +        ASSERT(src1 != src2);

This assertion is not necessary. Storing the same register twice is well defined, and works with storePair32.

[Angelos Oikonomopoulos]

Created attachment 449333 [details]
Patch

[Angelos Oikonomopoulos]

(In reply to Geza Lore from comment #5)
> Comment on attachment 449322 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449322&action=review
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:922
> > +        ASSERT(src1 != src2);
> 
> This assertion is not necessary. Storing the same register twice is well
> defined, and works with storePair32.

Ah. Removed, thanks!

[Radar WebKit Bug Importer]

<rdar://problem/87772960>

[Angelos Oikonomopoulos]

Ping.

[Zan Dobersek]

Comment on attachment 449333 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449333&action=review

> Source/JavaScriptCore/jit/AssemblyHelpersSpoolers.h:284
> +#if !(CPU(ARM64) || CPU(ARM))
> +        if constexpr (hasPairOp)
>              RELEASE_ASSERT_NOT_REACHED(); // unsupported architecture.
> +#endif

Honestly unguarded constexpr with additional `&& !isARM()` (if it existed) would be nicer.

[Angelos Oikonomopoulos]

Created attachment 453963 [details]
Patch

[Angelos Oikonomopoulos]

(In reply to Zan Dobersek from comment #10)
> Comment on attachment 449333 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449333&action=review
> 
> > Source/JavaScriptCore/jit/AssemblyHelpersSpoolers.h:284
> > +#if !(CPU(ARM64) || CPU(ARM))
> > +        if constexpr (hasPairOp)
> >              RELEASE_ASSERT_NOT_REACHED(); // unsupported architecture.
> > +#endif
> 
> Honestly unguarded constexpr with additional `&& !isARM()` (if it existed)
> would be nicer.

Makes sense. Added and used.

[EWS]

Committed r290907 (248137@main): <https://commits.webkit.org/248137@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453963 [details].