Remove WL extractor, merge into hacker revive#793
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy WL/greedy extractor path and makes the tiger-based extraction pipeline the sole extraction mechanism, updating the CLI, schedules, and profiling harness accordingly.
Changes:
- Removed
--use-tiger/--non-weakly-linearconfiguration plumbing and consolidated extraction through the tiger pipeline. - Added a
types_only()schedule for roundtrip checking to avoid canonicalization/always-run rewrites during that validation path. - Updated profiling treatments and cleaned up related code (including moving
is_dummy_ctxintofrom_egglog.rs).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Removes deprecated CLI flags and updates EggccConfig construction accordingly. |
| infra/profile.py | Drops removed CLI flags from benchmark invocations for tiger-related treatments. |
| dag_in_context/src/schedule.rs | Adds types_only() schedule and simplifies schedule construction; always runs non-weakly-linear in the parallel schedule. |
| dag_in_context/src/optimizations/hackers_delight.egg | Clarifies semantics around infinite loops / lowbit(0) in comments. |
| dag_in_context/src/linearity.rs | Removes the legacy linearity-checking helper module (greedy extractor-era). |
| dag_in_context/src/lib.rs | Deletes greedy-extractor integration and routes extraction/roundtrip through tiger; introduces local serialized_egraph()/has_debug_exprs(). |
| dag_in_context/src/greedy_dag_extractor.rs | Removes the legacy greedy DAG extractor implementation (and its tests). |
| dag_in_context/src/from_egglog.rs | Inlines is_dummy_ctx() to remove dependency on the deleted greedy extractor module. |
Comments suppressed due to low confidence (1)
dag_in_context/src/lib.rs:951
- When
has_debug_exprsis true, the code logs that it will “extract them instead of original program” and then returns early afterextract(). However,extract()no longer has a debug-expr extraction path (the previous greedy extractor handledDebugExpr). This likely makes the debug-expression visualization flow incorrect: the optimizer stops early but still extracts the normal functions. Consider either implementing DebugExpr extraction in the tiger path (or via egglog extraction over the DebugExpr relation) or removing the early-stop/log so behavior matches reality.
let has_debug_exprs = has_debug_exprs(&serialized);
if has_debug_exprs {
log::info!(
"Program has debug expressions, extracting them instead of original program."
);
}
let (iter_result, region_timings, extract_time) = extract(
eggcc_config,
&res,
batch.clone(),
&serialized,
should_maintain_linearity,
);
eggcc_extraction_time += extract_time;
eggcc_serialization_time += serialization_duration;
extract_region_timings.extend(region_timings);
// typecheck the program as a sanity check
iter_result.typecheck();
res = iter_result;
if has_debug_exprs {
log::info!("Program has debug expressions, stopping pass {}.", i);
return Ok((
res,
EggccTimeStatistics {
eggcc_extraction_time,
eggcc_serialization_time,
extract_region_timings,
},
));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
FTRobbin
left a comment
There was a problem hiding this comment.
No changes to the optimization rules themselves? Am I missing something?
0bdd0ff to
cbed2d2
Compare
FTRobbin
left a comment
There was a problem hiding this comment.
LGTM. Only a small question.
| (bop-of-type (Sub) (Base (IntT))) | ||
| (bop-of-type (Div) (Base (IntT))) | ||
| (bop-of-type (Mul) (Base (IntT))) | ||
| (bop-of-type (Smax) (Base (IntT))) |
There was a problem hiding this comment.
It was a bug our test suite revealed- we need those types for the extractor to work
No description provided.