Skip to content

Conversation

@preames
Copy link
Collaborator

@preames preames commented Dec 4, 2025

Add operator==, clarify invariants with an assert.

This does convert one llvm_unreachable into an assert which runs earlier in the computation. I don't believe the difference is observable outside debug builds, and should report the fault closer to introduction.

Add operator==, clarify invariants with an assert.

This does convert one llvm_unreachable into an assert which runs earlier
in the computation.  I don't believe the difference is observable outside
debug builds, and should report the fault closer to introduction.
}
if (CSRReg || CSROffset) {
CSRSavedLocation Loc(CSRReg, CSROffset);
auto It = CSRLocMap.find(CFI.getRegister());
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should try the insert instead of find here. If the insert fails, do the check to make sure the value we tried to insert matches what was already there.

This can be a follow up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See #170760

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@preames preames merged commit 87e14f5 into llvm:main Dec 4, 2025
10 of 11 checks passed
@preames preames deleted the pr-CSRSavedLocation-nfc branch December 4, 2025 21:36
honeygoyal pushed a commit to honeygoyal/llvm-project that referenced this pull request Dec 9, 2025
Add operator==, clarify invariants with an assert.  

This does convert one llvm_unreachable into an assert which runs earlier
in the computation. I don't believe the difference is observable outside
debug builds, and should report the fault closer to introduction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants