Fix/gc malloc cse o3#188
Merged
Merged
Conversation
…d as allocator functions At -O3, GVN/EarlyCSE treated repeated GC_malloc(N) calls with identical arguments as redundant and merged them into one shared allocation, since the renamed declarations carried no attributes distinguishing them from pure/read-only calls. This aliased distinct fields (e.g. two empty-array fields in a constructor), causing silent memory corruption and crashes once one field's backing store was later mutated independently. Set the LLVM dialect's memory_effects attribute (write to unmodeled memory) on these declarations so the optimizer can no longer assume two calls with the same arguments are interchangeable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ssion test for CSE bug
The allockind/memory-effects attributes added in prior commits (2cf7ec5, 45f140e, 2c9b1b4) correctly stop LLVM's own optimizer from merging GC_malloc calls, but they never mattered: the merge actually happens earlier, in MLIR's generic CSE pass (transform.cpp), which runs on the ts-dialect before GC_malloc calls even exist. `this.a = []` lowers to `ts.Constant` (empty const-array sentinel) followed by `ts.Cast` to a heap-backed ArrayType. CastOp was marked blanket `Pure`, so two structurally-identical Cast ops (same operand after the harmless Constant merge) looked redundant to CSE and got merged - even though this particular cast shape materializes a fresh GC_malloc call during later lowering (CastLogicHelper::castToArrayType's byValue branch). CSE folded both casts into one before that allocation ever existed in the IR, so the two class fields ended up aliasing the same backing array. Replace CastOp's `Pure` trait with a real MemoryEffectsOpInterface implementation: report Allocate/Write only for the ConstArrayType -> ArrayType shape (the one that allocates), leaving every other cast effect-free so normal CSE/DCE still applies to them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…inct boxed values
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix/gc malloc cse o3