Skip to content
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

bug[next]: Respect evaluation order in InlineCenterDerefLiftVars #1883

Merged
merged 6 commits into from
Feb 27, 2025

Conversation

tehrengruber
Copy link
Contributor

@tehrengruber tehrengruber commented Feb 20, 2025

Changes the InlineCenterDerefLiftVars pass to respect evaluation order by lazily evaluating the inlined values.

Consider the following case that is common in boundary conditions:

let(var, (↑deref)(it2))(if ·on_bc then 0 else ·var)

Then var should only be dereferenced in case ·on_bc evalutes to False. Previously we just evaluated all values unconditionally:

let(_icdlv_1, ·it)(if ·on_bc then 0 else _icdlv_1)

Now we instead create a 0-ary lambda function for _icdlv_1 and evaluate it when the value is needed.

let(_icdlv_1, λ() → ·it)(if ·on_bc then 0 else _icdlv_1())

Note that as a result we do evaluate the function multiple times. To avoid redundant recompuations usage of the common subexpression elimination is required.

@tehrengruber tehrengruber requested a review from havogt February 20, 2025 23:59
Copy link
Contributor

@havogt havogt left a comment

Choose a reason for hiding this comment

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

Can you replace bc by something like: conditionally evaluate?

@tehrengruber tehrengruber changed the title bug[next]: Respect bc in InlineCenterDerefLiftVars bug[next]: Respect evaluation order in InlineCenterDerefLiftVars Feb 27, 2025
@tehrengruber tehrengruber merged commit 587d107 into GridTools:main Feb 27, 2025
23 checks passed
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.

2 participants