Bug 235112 - AssemblyHelpersSpoolers: use load/store pair on ARMv7
Summary: AssemblyHelpersSpoolers: use load/store pair on ARMv7
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Angelos Oikonomopoulos
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-12 06:19 PST by Angelos Oikonomopoulos
Modified: 2022-03-07 13:56 PST (History)
9 users (show)

See Also:


Attachments
Patch (15.77 KB, patch)
2022-01-12 06:20 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (10.26 KB, patch)
2022-01-13 04:31 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (10.15 KB, patch)
2022-01-13 06:17 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (9.25 KB, patch)
2022-01-17 03:05 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (9.21 KB, patch)
2022-01-17 07:39 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (9.73 KB, patch)
2022-03-07 04:28 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Angelos Oikonomopoulos 2022-01-12 06:19:44 PST
AssemblyHelpersSpoolers: use load/store pair on ARMv7
Comment 1 Angelos Oikonomopoulos 2022-01-12 06:20:14 PST
Created attachment 448939 [details]
Patch
Comment 2 Angelos Oikonomopoulos 2022-01-13 04:31:47 PST
Created attachment 449044 [details]
Patch
Comment 3 Angelos Oikonomopoulos 2022-01-13 06:17:36 PST
Created attachment 449054 [details]
Patch
Comment 4 Angelos Oikonomopoulos 2022-01-17 03:05:08 PST
Created attachment 449322 [details]
Patch
Comment 5 Geza Lore 2022-01-17 04:14:50 PST
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.
Comment 6 Angelos Oikonomopoulos 2022-01-17 07:39:41 PST
Created attachment 449333 [details]
Patch
Comment 7 Angelos Oikonomopoulos 2022-01-17 07:43:09 PST
(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!
Comment 8 Radar WebKit Bug Importer 2022-01-19 08:46:42 PST
<rdar://problem/87772960>
Comment 9 Angelos Oikonomopoulos 2022-02-09 00:59:05 PST
Ping.
Comment 10 Zan Dobersek 2022-03-04 06:37:08 PST
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.
Comment 11 Angelos Oikonomopoulos 2022-03-07 04:28:56 PST
Created attachment 453963 [details]
Patch
Comment 12 Angelos Oikonomopoulos 2022-03-07 04:29:36 PST
(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.
Comment 13 EWS 2022-03-07 13:56:19 PST
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].