Skip to content

Fix dropal inline - #257

Merged
PalumboN merged 10 commits into
mainfrom
fix-dropal-inline
Aug 4, 2026
Merged

Fix dropal inline#257
PalumboN merged 10 commits into
mainfrom
fix-dropal-inline

Conversation

@PalumboN

@PalumboN PalumboN commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Fix #256
Fix #230

Now the compilation of BGMeteorBoard

DROpalCompiler recompileClass: BGMeteorBoard except: #().

finish on all (possible) cases:

1. hasEastOrWestIsland:
Error: Non-local return on inlined method breaks the semantics
2. northIslandsFor:row:
Error: Non-local return on inlined method breaks the semantics
3. findIsland:
Error: Non-local return on inlined method breaks the semantics
4. fillMaskStartingAt:stoppingAbove:ifFoundEnough:
Error: Non-local return on inlined method breaks the semantics
5. hasSouthIsland:
Error: Non-local return on inlined method breaks the semantics
6. islandsFor:
Error: Non-local return on inlined method breaks the semantics

and give these results:

[BenchmarkGameSuite new benchMeteor] bench.  
"ORIGINAL:         35 iterations in 5 seconds 88 milliseconds. 6.879 per second"
"DRUID (noinline): 10 iterations in 5 seconds 133 milliseconds. 1.948 per second"
"DRUID (inline):   12 iterations in 5 seconds 111 milliseconds. 2.348 per second"

We should profile the causes of:

  • 30% slowdown from the original Opal to DROpal without inlines
  • 20% speedup from DROpal without inlines to with them

what could be related to:

@PalumboN
PalumboN requested a review from guillep July 30, 2026 14:26
guillep
guillep previously approved these changes Aug 3, 2026

@guillep guillep 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.

Some small questions!

Comment thread Druid-Opal/DRLocalVariableInstructionScheluder.class.st Outdated
Comment thread Druid-Opal/DRLocalVariableInstructionScheluder.class.st Outdated
Comment thread Druid-Opal/DRMethodIRGenerator.class.st
Comment thread Druid-Opal/DRMethodIRGenerator.class.st Outdated
self dependencies do: [ :op |
(op hasUser: self) ifFalse: [
DRError signal:
Error signal:

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.

Why dis?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DRError does not exist 🤷 I checked the other validations and use Error. It looks like we didn't hit this problem for a long time

Comment thread Druid/DRInline.class.st
self class enabled ifFalse: [ ^ self ].

(cfg messageSends takeFirst: 5) do: [ :messageSend |
(cfg messageSends takeFirst: 3) do: [ :messageSend |

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.

3? 5? 7?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Image

Comment on lines +1721 to +1724
"Sends with blocks must keep the context (temporaries in the stack)
That is having all outer temporaries in the dependency-user chain.
If not, we don't need them, and the state is clear from previous context
This simplify the inlinings, the reason why the send has a inlineGenerator

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.

Really? even in the presence of temp vectors?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but during the compilation we need to track the def-use chain in the SSA. If not, variables used inside a block could be "dead-coded" before inlining.

The problem was that the implementation kept track a lot of trash (values that were not used), adding noise to the debugging.

Anyway, we need to revisit and re-think these decisions when optimizing temp vectors
#231
#251

Co-authored-by: Guille Polito <guillermopolito@gmail.com>
Co-authored-by: Nahuel Palumbo <nahuel.palumbo@gmail.com>
@PalumboN PalumboN changed the title [WIP] Fix dropal inline Fix dropal inline Aug 4, 2026
@PalumboN
PalumboN merged commit 4b291cd into main Aug 4, 2026
4 checks passed
@PalumboN
PalumboN deleted the fix-dropal-inline branch August 4, 2026 14:47
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.

Compiling BGMeteorBoard class Scope ids collisions?

2 participants