"Internal error" crash in Julia 1.7.0-beta4 when using the Gridap FEM library. #42264
Labels
compiler:optimizer
Optimization passes (mostly in base/compiler/ssair/)
regression
Regression in behavior compared to a previous version
Milestone
Comments
JeffBezanson
added
compiler:optimizer
|
Bisect picks 3635f04, which doesn't seem to directly make sense. It might be due to forming more union types, leading to more union splitting, eventually triggering this. |
This was referenced Sep 21, 2021
dkarrasch
pushed a commit
that referenced
this issue
Sep 23, 2021
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]>
|
Fixed by #42347 |
KristofferC
pushed a commit
that referenced
this issue
Sep 23, 2021
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]> (cherry picked from commit b5f3a99)
KristofferC
pushed a commit
that referenced
this issue
Sep 28, 2021
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]>
KristofferC
pushed a commit
that referenced
this issue
Oct 29, 2021
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]> (cherry picked from commit b5f3a99)
KristofferC
pushed a commit
that referenced
this issue
Oct 29, 2021
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]> (cherry picked from commit b5f3a99)
KristofferC
pushed a commit
that referenced
this issue
Nov 11, 2021
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]> (cherry picked from commit b5f3a99)
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this issue
Feb 22, 2022
…uliaLang#42347) The crash in JuliaLang#42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]>
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this issue
Mar 8, 2022
…uliaLang#42347) The crash in JuliaLang#42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]>
staticfloat
pushed a commit
that referenced
this issue
Dec 23, 2022
…42347) The crash in #42264 showed up when attempting to record the new value for an SSA rename during unionsplit inlining. Such a crash would only happen when `compact.idx == 1`, which is an unusual condition, because it implies that no statement of the original function had yet been processed (which is odd, because the call being inlined must have been processed by this point). As it turns out, there is currently exactly one case where this happens. If the inliner sees an `_apply_iterate` (e.g. from splatting) of something that is not a Tuple or SimpleVector (thus requiring calls to `iterate` to expand the value during `apply`), and if inference was able to determine the total length of the resulting iteration, a special case early inliner, will expand out the splat into explicit iterate calls. E.g. a call like: %r = tuple((x::Pair)...) will be expanded out to %a = iterate(x::Pair) %b = getfield(%a, 1) %c = getfield(%a, 2) %d = iterate(x, %c) %e = getfield(%d, 1) %f = getfield(%d, 2) iterate(x, %f) %r = tuple(%b, %e) where the inserted `iterate` calls may themselves be inlined. These newly inserted calls are "pending nodes" during the actual inlining. Thus, if the original apply call was the first statement of the function, these nodes would be processed before processing the statements in the function themselves. In particular, this investigation also shows that `compact.idx`, which is the current location in the function being inlined into is the wrong value to use for SSA renaming. Rather, we need to use the SSA value of the just-inserted statement. In the absence of pending nodes, these are equivalent concepts, but not for pending nodes. Fortunately the IncrementalCompact iterator provides the old SSA value for just this reason and in fact, non-UnionSplit inlining was already correct here. Thus, to fix the issue, simply adjust union splitting to work the same way as a non-UnionSplit inline. In coming up with the test case, an additional complication is that we currently do not perform this optimization for any calls where the apply call itself was unionsplit. It is somewhat unusual (which explains why we haven't seen this much) for the apply to not be union-split, but for the subsequent `iterate` to require union-splitting. In the problem case, what happened is that for two out of the three union cases, the `iterate` calls themselves would error, causing the resulting info object to look like a non-unionsplit apply call, triggering this issue. Co-authored-by: Keno Fischer <[email protected]> (cherry picked from commit b5f3a99)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We are experiencing a crash in Julia 1.7.0-beta4 with a simple MWE written with code from the Gridap FEM library.
See this issue for full details: gridap/Gridap.jl#657
If you need further help to dig deeper into the issue, just let me know.
Thanks for the help and the amazing Julia lang!
The text was updated successfully, but these errors were encountered: