Skip to content

cranelift: New InstructionData::map_values helper#8231

Merged
jameysharp merged 1 commit intobytecodealliance:mainfrom
jameysharp:map-values
Mar 25, 2024
Merged

cranelift: New InstructionData::map_values helper#8231
jameysharp merged 1 commit intobytecodealliance:mainfrom
jameysharp:map-values

Conversation

@jameysharp
Copy link
Contributor

This is like the DataFlowGraph::map_inst_values method, but that method mutably borrows self, so it goes to a fair bit of trouble to drop its borrows around callbacks so the mutable borrow can be used in the callback too.

This new helper only actually needs to borrow two public fields from the DFG: value_lists and jump_tables. Therefore it's possible to use other fields in the callback as long as the compiler can see all the fields being used in the same scope. That's good enough for everywhere we were using this pattern, so we can simplify this way.

The case which motivated this change isn't shown here: It isn't possible to call dfg.map_inst_values on every instruction in dfg.insts, because the borrow on dfg.insts prevents taking a mutable borrow on dfg. But we can call this new helper in that case.

This is like the DataFlowGraph::map_inst_values method, but that method
mutably borrows `self`, so it goes to a fair bit of trouble to drop its
borrows around callbacks so the mutable borrow can be used in the
callback too.

This new helper only actually needs to borrow two public fields from the
DFG: `value_lists` and `jump_tables`. Therefore it's possible to use
other fields in the callback as long as the compiler can see all the
fields being used in the same scope. That's good enough for everywhere
we were using this pattern, so we can simplify this way.

The case which motivated this change isn't shown here: It isn't possible
to call `dfg.map_inst_values` on every instruction in `dfg.insts`,
because the borrow on `dfg.insts` prevents taking a mutable borrow on
`dfg`. But we can call this new helper in that case.
@jameysharp jameysharp requested a review from a team as a code owner March 25, 2024 00:25
@jameysharp jameysharp requested review from elliottt and removed request for a team March 25, 2024 00:25
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Mar 25, 2024
Copy link
Member

@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

Awesome!

@jameysharp jameysharp added this pull request to the merge queue Mar 25, 2024
Merged via the queue into bytecodealliance:main with commit 9122595 Mar 25, 2024
@jameysharp jameysharp deleted the map-values branch March 25, 2024 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants