compiler: Fix two guard bugs exposed by buffering by ggorman · Pull Request #3061 · devitocodes/devito

ggorman · 2026-10-16T23:02:01Z

Two guard bugs that buffering exposes, each with a test against opt='noop':

if (false) was emitted. A condition SymPy folds to false (e.g. time < 0) printed if (false), which is not C99 without stdbool.h: a compile error. True and false now print as 1 and 0.
{time: true} guards. time >= 0 folds to true and gave a Cluster a guard distinct from no guard, so buffering created one buffer per "guard" for equivalent accesses. Guards now never sets a guard that reduces to true, whatever the entry point.

Tests: test_dimension.py::TestConditionalDimension::test_false_condition, test_buffering.py::test_true_condition_single_buffer, test_ir.py::TestGuards::test_true_never_set. Each fails without its source change.

An earlier GuardFactorNe change was dropped (4d820ac): no real program reaches a negated factor guard.

First of three stacked PRs: this one, then #3062 (copy-back skip rule) and #3063 (stale reads and lost writes).

🤖 Generated with Claude Code

GuardFactorNe inherited CondEq through GuardFactor ahead of CondNe, so it was an equality and printed `==`. Negating a subsampling guard thus reproduced the guard itself, selecting the timesteps on the subsampled grid instead of those off it

A condition SymPy folds to `false`, e.g. `time < 0`, was printed as `if (false)`, which is not C99 without `stdbool.h` and failed to compile

A condition such as `time >= 0` folds to `true` (`time` is nonnegative) and gave a Cluster the guard {time: true}, distinct from no guard. Buffering then created one buffer per guard for equivalent accesses

codecov · 2026-10-16T23:16:47Z

Codecov Report

❌ Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.23%. Comparing base (2707ac9) to head (4d820ac).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
devito/passes/iet/languages/C.py	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3061   +/-   ##
=======================================
  Coverage   84.23%   84.23%           
=======================================
  Files         258      258           
  Lines       57070    57125   +55     
  Branches     4874     4877    +3     
=======================================
+ Hits        48072    48122   +50     
- Misses       8170     8174    +4     
- Partials      828      829    +1

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	`68.51% <71.42%> (+<0.01%)`	⬆️
pytest-gpu-gcc-	`79.02% <98.14%> (+0.01%)`	⬆️
pytest-gpu-icx-	`78.94% <98.14%> (+0.01%)`	⬆️
pytest-gpu-nvc-nvidiaX	`69.08% <71.42%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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

seems sane

FabioLuporini · 2026-10-19T08:43:41Z

                guards = {d: mode(*v, evaluate=False) for d, v in guards.items()}

+            # A condition folded to `true` (e.g., `time >= 0`) imposes no guard
+            guards = {d: v for d, v in guards.items() if v is not sympy.true}


I think this should rather be fixed directly in class Guards, within devito/ir/support/guards.py: a guard, along any Dimension, should never be set (being pointless) if its value reduces to sympy.true

Done in 4c8c1cd: Guards.__init__ now drops any entry equal to sympy.true, so no entry point (constructor, andg, xandg, impose, subs, ...) can set one, and the filter in the guard pass is gone. Added TestGuards::test_true_never_set; the buffering test still covers the original case.

Drop such guards in `Guards` itself, at construction, rather than in the `guard` pass, so that no entry point (`andg`, `xandg`, `impose`, `subs`, ...) can set one

mloubout · 2026-10-19T11:53:04Z

+
+        ct = ConditionalDimension(name='ct', parent=time, factor=2)
+        cn = ConditionalDimension(name='cn', parent=time,
+                                  condition=Not(GuardFactor(ct)), relation='strict')


GuardFactor should not be used at this level this is an internal object, this needs to be a user API def, like Not(CondEq(...

Fair point, fixed in 763b8b2. GuardFactor is only ever built by the compiler from a ConditionalDimension(factor=...), and its negation only arises in buffering, when a copy-back is overwritten by a later one guarded by the factor dimension — so the test now goes through that: two writes to the same usave slot, one at the last timestep and one under factor=2, noop vs buffering (test_buffering.py::test_negated_factor_guard). Fails on main with if (time == 7 && time%tsubf == 0), passes with != 0. The old test is gone from test_dimension.py.

The negated subsampling guard is reached through buffering, when a copy-back is overwritten by a later one under a factor-based ConditionalDimension, so test it there instead of constructing the internal GuardFactor

mloubout · 2026-10-19T12:35:29Z

+    assert np.allclose(h.data[0], 12.)
+
+
+def test_negated_factor_guard():


that's an abuse of the API,

the usave isn't defined on tsub but on time

It saves two different values at the same steps which doesn't make sense.

You're right, it was contrived. I checked it against real code (recipes, devitohorizon, the devito examples and the devitopro demos/tests): every snapshot schedule saves on the subsampled dimension with a single writer, and the only same-slot two-writer case is the last-step save in your test_multi_cond_same_slot, which negates the CondEq guard, not the factor one, and is unaffected by the GuardFactorNe change. So nothing real reaches a negated factor guard, and I've dropped both that change and the test (4d820ac). The PR is now just the true/false guard fixes.

A ConditionalDimension whose condition folds to true must yield no Conditional in the generated code, under any relation

No user program reaches a negated factor guard: real snapshot schedules save on the subsampled dimension with a single writer, and the same-slot last-step save negates a CondEq instead. The test needed a contrived program to get there, so drop both

ggorman and others added 3 commits October 16, 2026 10:49

compiler: Make GuardFactorNe an inequality

effb773

GuardFactorNe inherited CondEq through GuardFactor ahead of CondNe, so it was an equality and printed `==`. Negating a subsampling guard thus reproduced the guard itself, selecting the timesteps on the subsampled grid instead of those off it

compiler: Print SymPy true/false as 1/0 in C

90dc7c1

A condition SymPy folds to `false`, e.g. `time < 0`, was printed as `if (false)`, which is not C99 without `stdbool.h` and failed to compile

compiler: Drop cluster guards folded to true

38e0aa4

A condition such as `time >= 0` folds to `true` (`time` is nonnegative) and gave a Cluster the guard {time: true}, distinct from no guard. Buffering then created one buffer per guard for equivalent accesses

ggorman added the compiler label Oct 16, 2026

This was referenced Oct 16, 2026

compiler: Skip copy-backs only if no write to the guard intervenes #3062

Draft

compiler: Fix stale reads and lost writes in buffering #3063

Draft

compiler: Skip copy-backs overwritten by a later one to the same slot #3050

Merged

mloubout reviewed Oct 19, 2026

View reviewed changes

FabioLuporini requested changes Oct 19, 2026

View reviewed changes

compiler: Never set a guard reducing to true

4c8c1cd

Drop such guards in `Guards` itself, at construction, rather than in the `guard` pass, so that no entry point (`andg`, `xandg`, `impose`, `subs`, ...) can set one

mloubout reviewed Oct 19, 2026

View reviewed changes

tests: Exercise GuardFactorNe through the user API

763b8b2

The negated subsampling guard is reached through buffering, when a copy-back is overwritten by a later one under a factor-based ConditionalDimension, so test it there instead of constructing the internal GuardFactor

mloubout reviewed Oct 19, 2026

View reviewed changes

ggorman added 2 commits October 19, 2026 12:40

tests: Check a true guard is never set through the user API

7bac902

A ConditionalDimension whose condition folds to true must yield no Conditional in the generated code, under any relation

mloubout approved these changes Oct 19, 2026

View reviewed changes

ggorman changed the title ~~compiler: Fix three guard bugs exposed by buffering~~ compiler: Fix two guard bugs exposed by buffering Oct 19, 2026

ggorman marked this pull request as ready for review October 19, 2026 13:32

ggorman requested a review from FabioLuporini October 19, 2026 13:32

Navigation Menu

Repository navigation

compiler: Fix two guard bugs exposed by buffering - #3061

compiler: Fix two guard bugs exposed by buffering#3061
ggorman wants to merge 7 commits into
mainfrom
fix-buffering-guards

ggorman commented Oct 16, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 16, 2026 •

edited

Loading

Uh oh!

mloubout left a comment

Uh oh!

FabioLuporini Oct 19, 2026

Uh oh!

ggorman Oct 19, 2026

Uh oh!

mloubout Oct 19, 2026

Uh oh!

ggorman Oct 19, 2026

Uh oh!

mloubout Oct 19, 2026

Uh oh!

ggorman Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		assert np.allclose(h.data[0], 12.)


		def test_negated_factor_guard():

Conversation

ggorman commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mloubout left a comment

Choose a reason for hiding this comment

Uh oh!

FabioLuporini Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

ggorman Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

mloubout Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

ggorman Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

mloubout Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

ggorman Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ggorman commented Oct 16, 2026 •

edited

Loading

codecov Bot commented Oct 16, 2026 •

edited

Loading