Skip to content

Conversation

@ftynse
Copy link
Contributor

@ftynse ftynse commented Jan 2, 2026

Add multiple verifiers on the invariants we always had but did not enforce
(potentially dropped by accident during refactoring). Sort symbols on
construction to ensure symbolically equal maps with different positions compare
as equal.

Signed-off-by: Alex Zinenko [email protected]

@ftynse ftynse marked this pull request as draft January 2, 2026 10:47
@ftynse ftynse force-pushed the users/ftynse/harden-index-mapping branch from 13fbc76 to 75a1d75 Compare January 2, 2026 11:14
@ftynse ftynse marked this pull request as ready for review January 2, 2026 11:15
Comment on lines 72 to 96
llvm::sort(symbolVector, [](Attribute lhs, Attribute rhs) {
auto lhsSymbol = dyn_cast<WaveSymbolAttr>(lhs);
auto lhsIndexSymbol = dyn_cast<WaveIndexSymbolAttr>(lhs);
auto lhsIterSymbol = dyn_cast<WaveIterSymbolAttr>(lhs);
auto rhsSymbol = dyn_cast<WaveSymbolAttr>(rhs);
auto rhsIndexSymbol = dyn_cast<WaveIndexSymbolAttr>(rhs);
auto rhsIterSymbol = dyn_cast<WaveIterSymbolAttr>(rhs);
// For different kinds of symbols, index symbols come before iter symbols,
// which come before regular symbols.
if (lhsIndexSymbol && !rhsIndexSymbol)
return true;
if (!lhsIndexSymbol && rhsIndexSymbol)
return false;
if (lhsIterSymbol && rhsSymbol)
return true;
if (lhsSymbol && !rhsSymbol)
return false;
if (lhsSymbol && rhsSymbol)
return lhsSymbol.getName() < rhsSymbol.getName();
if (lhsIndexSymbol && rhsIndexSymbol)
return lhsIndexSymbol.getValue() < rhsIndexSymbol.getValue();
if (lhsIterSymbol && rhsIterSymbol)
return lhsIterSymbol.getName() < rhsIterSymbol.getName();
assert(false && "unhandled symbol comparison");
});
Copy link
Contributor

Choose a reason for hiding this comment

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

this may be easier to read.

Suggested change
llvm::sort(symbolVector, [](Attribute lhs, Attribute rhs) {
auto lhsSymbol = dyn_cast<WaveSymbolAttr>(lhs);
auto lhsIndexSymbol = dyn_cast<WaveIndexSymbolAttr>(lhs);
auto lhsIterSymbol = dyn_cast<WaveIterSymbolAttr>(lhs);
auto rhsSymbol = dyn_cast<WaveSymbolAttr>(rhs);
auto rhsIndexSymbol = dyn_cast<WaveIndexSymbolAttr>(rhs);
auto rhsIterSymbol = dyn_cast<WaveIterSymbolAttr>(rhs);
// For different kinds of symbols, index symbols come before iter symbols,
// which come before regular symbols.
if (lhsIndexSymbol && !rhsIndexSymbol)
return true;
if (!lhsIndexSymbol && rhsIndexSymbol)
return false;
if (lhsIterSymbol && rhsSymbol)
return true;
if (lhsSymbol && !rhsSymbol)
return false;
if (lhsSymbol && rhsSymbol)
return lhsSymbol.getName() < rhsSymbol.getName();
if (lhsIndexSymbol && rhsIndexSymbol)
return lhsIndexSymbol.getValue() < rhsIndexSymbol.getValue();
if (lhsIterSymbol && rhsIterSymbol)
return lhsIterSymbol.getName() < rhsIterSymbol.getName();
assert(false && "unhandled symbol comparison");
});
auto cmp = [](Attribute x) {
return TypeSwitch<Attribute, std::pair<int, StringRef>>(x)
.Case([](WaveIndexSymbolAttr a) {
return std::make_pair(1, a.getValue());
})
.Case(
[](WaveIterSymbolAttr a) { return std::make_pair(2, a.getName()); })
.Case([](WaveSymbolAttr a) { return std::make_pair(3, a.getName()); })
.DefaultUnreachable("unhandled symbol comparison");
};
llvm::sort(symbolVector,
[&](Attribute lhs, Attribute rhs) { return cmp(lhs) < cmp(rhs); });
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, I did it a bit differently since getValue on index symbols is not a string.

Copy link
Contributor

@tyb0807 tyb0807 left a comment

Choose a reason for hiding this comment

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

LGTM

@ftynse ftynse force-pushed the users/ftynse/harden-index-mapping branch from 75a1d75 to 99a425a Compare January 2, 2026 16:27
@ftynse ftynse requested review from tgymnich and tyb0807 January 2, 2026 16:27
@ftynse
Copy link
Contributor Author

ftynse commented Jan 2, 2026

There actually was a bug in indexing ;) PTAL

@ftynse ftynse force-pushed the users/ftynse/harden-index-mapping branch from 99a425a to 2a65b2e Compare January 2, 2026 16:32
Add multiple verifiers on the invariants we always had but did not enforce
(potentially dropped by accident during refactoring). Sort symbols on
construction to ensure symbolically equal maps with different positions compare
as equal. Systematically drop unused symbols on construction.

Signed-off-by: Alex Zinenko <[email protected]>
@ftynse ftynse force-pushed the users/ftynse/harden-index-mapping branch from 11fa9b0 to 90e49eb Compare January 5, 2026 10:57
@ftynse ftynse merged commit a95c883 into main Jan 5, 2026
15 checks passed
@ftynse ftynse deleted the users/ftynse/harden-index-mapping branch January 5, 2026 11:45
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.

4 participants