Skip to content

Conversation

@MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Jan 15, 2026

While working on MaD support for barriers/barrier guards for C/C++ I discovered a bug related to indirect barrier guards. This PR fixes that bug at the cost of:

  • Exposing a predicate that was private in the shared Guards library, and
  • Losing a small bit of smartness in barrier guards related to stuff like:
    if(x == y && clean(x)) {
      sink(y); // okay because `clean` is a barrier and `x == y`
    }

The problem is that (as @aschackmull also discovered when we were porting C++ to the shared SSA library's dataflow integration layer) we cannot relate an SsaDefinition to an Instruction without an indirection index. So on something like:

void foo(int* p) {
  if (validate_indirection(p)) {
    sink(*p);
  }
}

So when guardChecksInstr says that p is a barrier guard with indirection 1 guardChecksDef finds the associated Ssa definition for p (and not its indirection, *p, since guardReadsSsaVar doesn't have access to the indirection index) and places a barrier on &p, p, and *p instead of just *p like it was intended.

I can imagine better fixes for this. In particular:

  • Thread an additional language-supplied data column through the guards library and make it available for getARead.
  • C/C++ instantiates the shared Guards library with AstNode = (Instruction, int) instead of AstNode = Instruction like we do currently. This should be totally doable (albeit with another breaking change to the the barrier guards library for C/C++ 😭)

@aschackmull do you have any thoughts on this?

@MathiasVP MathiasVP changed the title Fix conflation in guards C++: Fix conflation in barrier guards Jan 15, 2026
@github-actions github-actions bot added the C++ label Jan 15, 2026
@aschackmull
Copy link
Contributor

The fix as-is looks reasonable, I think. I don't think the two alternatives you list sound very appealing.

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