Fix a bug in IR lowering (#1701)

The underlying problem here is that our `SharedIRBuilder` (which currently owns the "global" value-numbering map) has a subtle invariant ("subtle" in the sense of "dangerous and bad"). The value-numbering map stores `IRInst`s for things like constants and types, and if those instructions end up getting modified or deleted (deleting an instruction currently runs its destructor but does not free the pool-allocated memory), then it is possible for the computed hash code for an instruction to no longer match what it was when it was inserted.

The trigger in this case was a use of the `IRInst::removeAndDeallocate()` operation inside of the AST-to-IR lowering pass, which uses a single `SharedIRBuilder`. If that `removeAndDeallocate()` happens to apply to a value in the value-numbering map, then it risks breaking the next time the map gets rehashed.

The short-term fix here is simple: never try to delete an instruction during IR lowering, even if it is known to be unused. Instead, we can rely on the subsequent DCE pass to eliminate the instruction.

A longer-term fix here would involve fixing our entire strategy around value numbering. We know we need to do that, but that would be a big enough change that it couldn't be pursued as part of a simple bug fix like this.

1 files changed, 0 insertions, 2 deletions

diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index 159b2478c..58a5b0ed1 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -6048,8 +6048,6 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
                     // function types.
                     auto reqType = requirementVal->getFullType();
                     entry->setRequirementVal(reqType);
-                    if (!requirementVal->hasUses())
-                        requirementVal->removeAndDeallocate();
                     break;
                 }
                 default: