Skip to content

compiler: Improve scalar aliases detection and scheduling#2858

Merged
mloubout merged 4 commits intomainfrom
alias-multi-cond
Mar 5, 2026
Merged

compiler: Improve scalar aliases detection and scheduling#2858
mloubout merged 4 commits intomainfrom
alias-multi-cond

Conversation

@mloubout
Copy link
Contributor

@mloubout mloubout commented Feb 25, 2026

Quite a few more changes that I would have liked but this avoid being too drastic. This basically make scalar aliases more reobust

  • Remove guards from scalar aliases used in multiple clusters to avoid issue with scope of variable
  • No longer allow scalar aliases that depend on a condition
  • Make aliases const (tiny change but bit better technically)
  • Small fix on lifting that prevents detaching scalar clusters from cluster with same guard
  • Tiny change to cse dtype due to numpy integer upcasting

@mloubout mloubout force-pushed the alias-multi-cond branch 2 times, most recently from 19254fd to 6af1a6b Compare February 25, 2026 16:21
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@mloubout mloubout changed the title compiler: prevent undefined Temp through global init compiler: Prevent undefined Temp through global init Feb 25, 2026
@mloubout mloubout force-pushed the alias-multi-cond branch 3 times, most recently from e2045e5 to b859dc4 Compare February 25, 2026 17:02
@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.03%. Comparing base (28ded5a) to head (eb3b4ea).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2858      +/-   ##
==========================================
+ Coverage   78.96%   79.03%   +0.06%     
==========================================
  Files         248      248              
  Lines       51089    51210     +121     
  Branches     4422     4426       +4     
==========================================
+ Hits        40342    40473     +131     
+ Misses       9938     9926      -12     
- Partials      809      811       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mloubout mloubout force-pushed the alias-multi-cond branch 10 times, most recently from 7fa5e9a to 8c0a82b Compare February 26, 2026 17:19
@mloubout mloubout changed the title compiler: Prevent undefined Temp through global init compiler: Improve scalar aliases detection and scheduling Feb 26, 2026
@mloubout mloubout force-pushed the alias-multi-cond branch 3 times, most recently from 42387e7 to 1a3141e Compare February 26, 2026 20:04

lifted.append(c.rebuild(ispace=ispace, properties=properties, guards=guards))
_lifted = c.rebuild(ispace=ispace, properties=properties, guards=guards)
if guards and clusters[max(n-1, 0)].guards != guards and _lifted.is_scalar:
Copy link
Contributor

Choose a reason for hiding this comment

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

this is very convoluted

Copy link
Contributor

Choose a reason for hiding this comment

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

if we don't pull outside of a guard (as I just wrote above) extra checks here might not be required after all

is_cond = any(isinstance(d, (SubsamplingFactor, ConditionalDimension))
for d in pivot.free_symbols)
if meta.guards and is_cond and nclusters > 1:
# Scalar alias that depends on a guard, unsafe to lift out of the guard
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the wrong place, they shouldn't have been collected in the first place.

Instead, you can drop this thing and rather add new rule here: https://github.com/devitocodes/devito/blob/main/devito/passes/clusters/aliases.py#L299-L300

Copy link
Contributor

@FabioLuporini FabioLuporini left a comment

Choose a reason for hiding this comment

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

LGTM

@mloubout mloubout force-pushed the alias-multi-cond branch 2 times, most recently from e54b7b0 to 3601883 Compare March 5, 2026 16:16
@mloubout mloubout merged commit 04373ef into main Mar 5, 2026
61 of 63 checks passed
@mloubout mloubout deleted the alias-multi-cond branch March 5, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants