Skip to content

[NFC] LocalGraph: Move definition to logical place - #6045

Merged
kripken merged 2 commits into
mainfrom
lg.fast
Oct 24, 2023
Merged

kripken merged 2 commits into
mainfrom
lg.fast

Conversation

@kripken

@kripken kripken commented Oct 23, 2023 •

Copy link
Copy Markdown
Member

allGets was declared in a scope that kept it alive for all blocks, and at
the end of the loop we clear the gets for a particular block. That's clumsy,
and makes a followup harder, so this PR moves it to the natural place for it.
(That is, it moves it to the scope that handles a particular block, and removes
the manual clearing-out of the get at the end of the loop iteration.) Optimizing
compilers are smart enough to be efficient about stack allocations
of objects inside loops anyhow (which I measured).

Helps #6042.

@kripken
kripken requested a review from tlively October 23, 2023 23:49

@tlively tlively left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay for less mutable state!

@kripken
kripken merged commit cce277f into main Oct 24, 2023
@kripken
kripken deleted the lg.fast branch October 24, 2023 17:17
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
allGets was declared in a scope that kept it alive for all blocks, and at
the end of the loop we clear the gets for a particular block. That's clumsy,
and makes a followup harder, so this PR moves it to the natural place for it.
(That is, it moves it to the scope that handles a particular block, and removes
the manual clearing-out of the get at the end of the loop iteration.) Optimizing
compilers are smart enough to be efficient about stack allocations
of objects inside loops anyhow (which I measured).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants