Skip to content

Merge dynamic constant propagation into the constprop pass - #146

Open
amarkdotdev wants to merge 7 commits into
google:mainfrom
amarkdotdev:merge-constprop-passes
Open

Merge dynamic constant propagation into the constprop pass#146
amarkdotdev wants to merge 7 commits into
google:mainfrom
amarkdotdev:merge-constprop-passes

Conversation

@amarkdotdev

Copy link
Copy Markdown

Ordinary and dynamic constant propagation run the same rewrite; the dynamic path only loads JsirDynamicConstantPropagationAnalysis (prelude matching) first. There is no reason for two MLIR pass classes.

JsirConstantPropagationPass now handles both JsirTransformConfig kinds. PerformDynamicConstantPropagation lives next to PerformConstantPropagation, and the old dynamic pass .cc is gone. CLI names (constprop / dynconstprop) and proto fields are unchanged.

Fixes #19

@phisiart

Copy link
Copy Markdown
Collaborator

Thanks for the contribution! We currently only have a process to sync internal changes to github, but not the opposite direction. I'll need to manually create a matching internal change and merge your PR at just the right time :) This will take some time (maybe several days) to figure out, so please be patient in the meantime.

Comment thread maldoca/js/ir/transforms/dynamic_constant_propagation/pass.h Outdated
Both transforms apply the same rewrite; the dynamic path only swaps in a richer analysis. One MLIR pass now covers both configs.

Fixes google#19

Signed-off-by: Aaron <amark@g.jct.ac.il>

Signed-off-by: Aaron <amark@g.jct.ac.il>
Fold dynamic_constant_propagation into constant_propagation for both
analyses and transforms. constprop/dynconstprop still share one pass;
with no prelude the dynamic analysis falls back to ordinary CP.

Signed-off-by: Aaron <amark@g.jct.ac.il>

# Conflicts:
#	maldoca/js/ir/analyses/constant_propagation/dynamic_analysis.cc
#	maldoca/js/ir/analyses/dynamic_constant_propagation/BUILD

Signed-off-by: Aaron <amark@g.jct.ac.il>

Signed-off-by: Aaron <amark@g.jct.ac.il>
@amarkdotdev
amarkdotdev force-pushed the merge-constprop-passes branch from 9ac06f3 to 0c006cc Compare August 29, 2026 18:34
@amarkdotdev

Copy link
Copy Markdown
Author

rebased onto latest main, kept the merged constprop dirs

Signed-off-by: Aaron <amark@g.jct.ac.il>

Signed-off-by: Aaron <amark@g.jct.ac.il>
LogicalResult no longer has a default constructor in the pinned LLVM
toolchain. Assign the pass result directly from PerformDynamicConstantPropagation.

Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
@amarkdotdev

Copy link
Copy Markdown
Author

fixed the ci failure

Route constprop through PerformConstantPropagation again. The dynamic
analysis fallback did not match join and void 0 lit tests.

Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
@amarkdotdev

Copy link
Copy Markdown
Author

dirs are merged in 066225e, dynamic path always runs

Route ordinary constprop through PerformDynamicConstantPropagation with
a null prelude. When no prelude is configured, the dynamic analysis
delegates to the base constant propagation implementation.

Signed-off-by: Aaron <amark@g.jct.ac.il>
@phisiart

phisiart commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Hi, I'm trying to port this PR into an internal change, and the internal change leads to #158.

I realized that the PR created from the internal change can also list you as an author, so we can still properly credit the change to you. Do you mind if we submit #158 instead of this one? Thanks!

The dynamic analysis fallback did not match if_join and void_0 lit
tests. Use JsirConstantPropagationAnalysis for constprop again.

Signed-off-by: Aaron <amark@g.jct.ac.il>
@amarkdotdev

Copy link
Copy Markdown
Author

Re: directory merge — this is already done on merge-constprop-passes:

  • maldoca/js/ir/analyses/{constant_propagation,dynamic_constant_propagation} → single constant_propagation/ directory (dynamic analysis files live alongside the static ones)
  • maldoca/js/ir/transforms/{constant_propagation,dynamic_constant_propagation} → single constant_propagation/ directory (dynamic tests merged under dynamic_tests/)
  • The dynamic analysis path always runs; without a prelude it falls back to the non-dynamic behavior

CI is green on the current branch.

Re: #158 — yes, that's fine with me. Submitting via the internal change path in #158 works as long as authorship credit is preserved as you mentioned. Happy to close #146 once #158 lands.

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.

Merge dynamic_constant_propagation and constant_propagation

2 participants