Skip to content

[mono] Always store to allocas in OP_LLVM_OUTARG_VT #64303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 27, 2022

Conversation

imhameed
Copy link
Contributor

OP_LLVM_OUTARG_VT will, for some argument passing conventions, create an
alloca and mirror its source SSA value into this alloca. If such an
OP_LLVM_OUTARG_VT is first encountered in a basic block that does not
contain the definition of the SSA value being mirrored, then sibling
basic blocks (e.g. a loop body that may sometimes be skipped) can use
garbage data if they also have OP_LLVM_OUTARG_VT opcodes referring to
the same SSA values.

This commit works around this by unconditionally storing the source
OP_LLVM_OUTARG_VT value into its associated alloca. The resulting IR
would be a little easier to read if we eagerly stored to an alloca
mirror exactly once at each value's definition, but that would require
more work to implement.

Fixes JIT/SIMD/VectorExp_ro/VectorExp_ro. See #64179.

@ghost ghost assigned imhameed Jan 25, 2022
@imhameed imhameed added the runtime-mono specific to the Mono runtime label Jan 25, 2022
OP_LLVM_OUTARG_VT will, for some argument passing conventions, create an
alloca and mirror its source SSA value into this alloca. If such an
OP_LLVM_OUTARG_VT is first encountered in a basic block that does not
contain the definition of the SSA value being mirrored, then sibling
basic blocks (e.g. a loop body that may sometimes be skipped) can use
garbage data if they also have OP_LLVM_OUTARG_VT opcodes referring to
the same SSA values.

This commit works around this by unconditionally storing the source
OP_LLVM_OUTARG_VT value into its associated alloca. The resulting IR
would be a little easier to read if we eagerly stored to an alloca
mirror exactly once at each value's definition, but that would require
more work to implement.
@imhameed imhameed force-pushed the mono-llvm-update-memory-mirror branch from 0b4a806 to a36763f Compare January 26, 2022 00:34
@vargaz
Copy link
Contributor

vargaz commented Jan 26, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@imhameed imhameed merged commit cfd15d5 into dotnet:main Jan 27, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Codegen-LLVM-mono runtime-mono specific to the Mono runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants