Skip to content

[compiler] Allow reordering more types of instructions - #30653

Closed
josephsavona wants to merge 3 commits into
gh/josephsavona/35/basefrom
gh/josephsavona/35/head
Closed

[compiler] Allow reordering more types of instructions#30653
josephsavona wants to merge 3 commits into
gh/josephsavona/35/basefrom
gh/josephsavona/35/head

Conversation

@josephsavona

@josephsavona josephsavona commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

Stack from ghstack (oldest at bottom):

Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

  • ArrayExpression and ObjectExpression can be reordered if they have no items
  • PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
  • LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
  • StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

* ArrayExpression and ObjectExpression can be reordered if they have no items
* PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
* LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
* StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

[ghstack-poisoned]
@vercel

vercel Bot commented Aug 9, 2024

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 9:38pm

josephsavona added a commit that referenced this pull request Aug 9, 2024
Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

* ArrayExpression and ObjectExpression can be reordered if they have no items
* PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
* LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
* StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

ghstack-source-id: 22422b5
Pull Request resolved: #30653
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 9, 2024
Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

* ArrayExpression and ObjectExpression can be reordered if they have no items
* PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
* LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
* StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

[ghstack-poisoned]
josephsavona added a commit that referenced this pull request Aug 9, 2024
Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

* ArrayExpression and ObjectExpression can be reordered if they have no items
* PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
* LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
* StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

ghstack-source-id: 1b42599
Pull Request resolved: #30653
Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

* ArrayExpression and ObjectExpression can be reordered if they have no items
* PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
* LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
* StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

[ghstack-poisoned]
josephsavona added a commit that referenced this pull request Aug 9, 2024
Updates InstructionReordering (still off by default) to allow reordering more types of instructions:

* ArrayExpression and ObjectExpression can be reordered if they have no items
* PropertyLoad can be reordered if the object is not mutable at the given instruction, and if the object is single-use (this avoid reordering in cases where the propertyload is part of a methodcall)
* LoadLocal can be reordered if the temporary is used once (ie not as part of a methodcall) and if the value being loaded is not subsequently reassigned
* StoreLocal can be reordered if the variable being assigned is only used once, and the rvalue is not later reassigned.

We can further relax these rules but already it improves compilation output quite a bit, per the fixtures.

ghstack-source-id: 896807c
Pull Request resolved: #30653
@mvitousek

Copy link
Copy Markdown
Contributor

Mostly seems good, but consider this case (link is to this PR's playground). Unclear to me what's going wrong here--this example passes on master, and here it doesn't pass for very unclear reasons (bad error message at the least).

In general I was trying to make sure that the logic for LoadLocal here is robust enough to handle a reassignment in a nested scope--maybe that's part of what's going wrong with this example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants