Bug 234952

Summary:

[RISCV64] riscv64 backend should lower offlineasm instructions

Product:

WebKit

Reporter:

Zan Dobersek <zan>

Component:

New Bugs

Assignee:

Zan Dobersek <zan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, pmatos, saam, tzagallo, webkit-bug-importer, xan.lopez, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

235164

Bug Blocks:

237775

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Description Zan Dobersek 2022-01-07 02:55:30 PST

[RISCV64] riscv64 backend should lower offlineasm instructions

Comment 1 Zan Dobersek 2022-01-07 02:56:19 PST

Created attachment 448577 [details]
Patch

Comment 2 Zan Dobersek 2022-01-09 23:59:25 PST

Created attachment 448723 [details]
Patch

With additional shift-amount validation.

Comment 3 Yusuke Suzuki 2022-01-11 23:18:34 PST

Comment on attachment 448723 [details]
Patch

r=me

Comment 4 EWS 2022-01-11 23:53:52 PST

Committed r287912 (245945@main): <https://commits.webkit.org/245945@main>

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

Comment 5 Radar WebKit Bug Importer 2022-01-11 23:54:17 PST

<rdar://problem/87440612>

Comment 6 WebKit Commit Bot 2022-01-12 16:28:39 PST

Re-opened since this is blocked by bug 235164

Comment 7 Zan Dobersek 2022-01-13 02:17:19 PST

With regards to the regression that initiated the rollout in #235164, the only thing that could affect ARM64/ARM64E is the additional lowering of misplaced addresses for any baddi*, bsubi* and bmuli* instructions. But these should already be lowered by the time that Sequence.getModifiedListARM64() calls riscLowerMisplacedAddresses(), due to already calling riscLowerSimpleBranchOps() and riscLowerHardBranchOps64() before that.

Don't know. I can implement Sequence.lowerMisplacedAddressesRISCV64() and manually lower these instructions there, not affecting any other architecture.

Comment 8 Zan Dobersek 2022-01-13 04:43:34 PST

Created attachment 449048 [details]
Patch

Comment 9 Zan Dobersek 2022-01-13 04:45:43 PST

Comment on attachment 449048 [details]
Patch

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

> Source/JavaScriptCore/offlineasm/riscv64.rb:406
> +def riscv64LowerMisplacedAddresses(list)
> +    newList = []
> +    list.each {
> +        | node |
> +        if node.is_a? Instruction
> +            case node.opcode
> +            when /^baddi/, /^bsubi/, /^bmuli/
> +                postInstructions = []
> +                newList << node.riscCloneWithOperandsLowered(newList, postInstructions, "i")
> +                newList += postInstructions

These specific instructions are now handled in a separate, RISCV64-specific lowering pass. Didn't use Instruction.lowerMisplacedAddressesRISCV64() to keep the lowering passes uniformly implemented in helper functions.

Comment 10 Yusuke Suzuki 2022-01-13 15:00:28 PST

Comment on attachment 449048 [details]
Patch

r=me

Comment 11 EWS 2022-01-13 20:38:47 PST

Committed r288007 (246033@main): <https://commits.webkit.org/246033@main>

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