Bad PhiC node placement/usage corrupts gc-preserve use #34247
Assignees
Milestone
Comments
|
How coincidental: this is a duplicate of #34229 that I opened the day before. |
|
I'm thinking the way to fix this would be to allow ssa uses across try/catch regions, coupled with an LLVM pass that either manually spills all state to the stack or outlines the body of the try/catch block. |
|
Hm, yeah, I think yuyichao was exploring something like that in #24573, but I don't know how feasible that is. In theory, the C standard says that LLVM is supposed to be OK with any variable that isn't modified in the try block being correct afterwards, and we seemed to be mostly OK for some years with assuming that. I still hope that someday we could move to using the lower-cost and more correct invoke instruction handling though. |
|
FWIW, bisected to cd92f60 (the upgrade to LLVM 8.0.1). |
Keno
added a commit
that referenced
this issue
Jan 14, 2020
This fixes #34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend.
Keno
added a commit
that referenced
this issue
Jan 15, 2020
This fixes #34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend.
KristofferC
pushed a commit
that referenced
this issue
Jan 15, 2020
This fixes #34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend. (cherry picked from commit 07a16d6)
KristofferC
pushed a commit
that referenced
this issue
Jan 15, 2020
This fixes #34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend. (cherry picked from commit 07a16d6)
KristofferC
pushed a commit
that referenced
this issue
Jan 17, 2020
This fixes #34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend. (cherry picked from commit 07a16d6)
KristofferC
pushed a commit
that referenced
this issue
Apr 11, 2020
This fixes #34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend.
BioTurboNick
pushed a commit
to BioTurboNick/julia
that referenced
this issue
Apr 13, 2020
This fixes JuliaLang#34247 by changing the way gc preserve is lowered. Instead of lowering it in a macro, lower it in the frontend. This allows us to use an SSA value directly for the return token of the gc begin expression. This bypasses the slot-renaming pass of the compiler, thus preventing the compiler from trying to save and restore the token. Of course, this kind of code would generally not be legal (because it uses an SSA value outside of the regular domination relation), but since this is a julia restriction, not an LLVM restriction, we can simply exempt gc_begin tokens from this particular validation. This works fine at the LLVM level also, because it doesn't have this particular restriction. It also doesn't have the same correctness problems as doing the same for non-token values, as the tokens get lowered away by the try/catch lowering before reaching the LLVM backend. (cherry picked from commit 07a16d6)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Report: https://github.com/maleadt/BasePkgEvalReports/blob/0bb12607c245153c8e7215788e580d43d7d2d51a/pkgeval-9bd498d_vs_2e6715c/logs/Strided/1.4.0-DEV-082f446f52.log
The text was updated successfully, but these errors were encountered: