-
Notifications
You must be signed in to change notification settings - Fork 47k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[compiler][hir] Correctly remove non-existent terminal preds when pruning labels #30076
Conversation
…ning labels [ghstack-poisoned]
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Comparing: 133ada7...01693b6 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
@@ -67,4 +67,14 @@ export function pruneUnusedLabelsHIR(fn: HIRFunction): void { | |||
fn.body.blocks.delete(fallthroughId); | |||
rewrites.set(fallthroughId, labelId); | |||
} | |||
|
|||
for (const [_, block] of fn.body.blocks) { | |||
for (const pred of [...block.preds]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: let’s avoid the array copy here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a small suggestion
…ds when pruning labels" Missed this initially in `pruneUnusedLabelsHIR`. It wasn't an active bug as `preds` wasn't referenced by later passes, until #30079 [ghstack-poisoned]
…ds when pruning labels" Missed this initially in `pruneUnusedLabelsHIR`. It wasn't an active bug as `preds` wasn't referenced by later passes, until #30079 [ghstack-poisoned]
Stack from ghstack (oldest at bottom):
Missed this initially in
pruneUnusedLabelsHIR
. It wasn't an active bug aspreds
wasn't referenced by later passes, until #30079