Bug 234952 - [RISCV64] riscv64 backend should lower offlineasm instructions
Summary: [RISCV64] riscv64 backend should lower offlineasm instructions
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: Zan Dobersek
URL:
Keywords: InRadar
Depends on: 235164
Blocks: 237775
  Show dependency treegraph
 
Reported: 2022-01-07 02:55 PST by Zan Dobersek
Modified: 2022-03-11 05:33 PST (History)
11 users (show)

See Also:


Attachments
Patch (128.67 KB, patch)
2022-01-07 02:56 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (129.17 KB, patch)
2022-01-09 23:59 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (128.79 KB, patch)
2022-01-13 04:43 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].