-
Notifications
You must be signed in to change notification settings - Fork 805
Correct dynamic_cast when value might be null #22241
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
Conversation
Signed-off-by: James Newling <[email protected]>
|
Odd - do you have IR for this or what was failing? It should never be legal to get to StreamToHAL with null affinities on ops that require them at that phase and I want to make sure we don't have a regression above it. |
|
Actually I'm seeing the assert error on main. Final IR before crash is below. @benvanik note, I'm not passing a device to iree-compile. // -----// IR Dump Before ConvertToHALPass (iree-hal-conversion) //----- //
module {
util.func public @simpleDAG() attributes {iree.abi.stub, iree.reflection = {iree.abi.declaration = "sync func @simpleDAG() -> ()"}} {
util.call @_simpleDAG() : () -> ()
util.return
}
util.func private @_simpleDAG() {
%c0 = arith.constant 0 : index
%c1091567616_i32 = arith.constant 1091567616 : i32
%c4 = arith.constant 4 : index
%c1077936128_i32 = arith.constant 1077936128 : i32
%result, %result_timepoint = stream.resource.alloca uninitialized : !stream.resource<external>{%c4} => !stream.timepoint
%0 = stream.cmd.execute await(%result_timepoint) => with(%result as %arg0: !stream.resource<external>{%c4}) {
stream.cmd.fill %c1077936128_i32, %arg0[%c0 for %c4] : i32 -> !stream.resource<external>{%c4}
} => !stream.timepoint
%1 = stream.timepoint.await %0 => %result : !stream.resource<external>{%c4}
%2 = stream.tensor.export %1 : tensor<f32> in !stream.resource<external>{%c4} -> tensor<f32>
check.expect_almost_eq(%2, %2) : tensor<f32>
%result_0, %result_timepoint_1 = stream.resource.alloca uninitialized : !stream.resource<external>{%c4} => !stream.timepoint
%3 = stream.cmd.execute await(%result_timepoint_1) => with(%result_0 as %arg0: !stream.resource<external>{%c4}) {
stream.cmd.fill %c1091567616_i32, %arg0[%c0 for %c4] : i32 -> !stream.resource<external>{%c4}
} => !stream.timepoint
%4 = stream.timepoint.await %3 => %result_0 : !stream.resource<external>{%c4}
%5 = stream.tensor.export %4 : tensor<f32> in !stream.resource<external>{%c4} -> tensor<f32>
check.expect_almost_eq(%5, %5) : tensor<f32>
util.return
}
util.func public @simpleHorizontal() attributes {iree.abi.stub, iree.reflection = {iree.abi.declaration = "sync func @simpleHorizontal() -> ()"}} {
util.call @_simpleHorizontal() : () -> ()
util.return
}
util.func private @_simpleHorizontal() {
%c0 = arith.constant 0 : index
%c1090519040_i32 = arith.constant 1090519040 : i32
%c4 = arith.constant 4 : index
%c1091567616_i32 = arith.constant 1091567616 : i32
%result, %result_timepoint = stream.resource.alloca uninitialized : !stream.resource<external>{%c4} => !stream.timepoint
%0 = stream.cmd.execute await(%result_timepoint) => with(%result as %arg0: !stream.resource<external>{%c4}) {
stream.cmd.fill %c1091567616_i32, %arg0[%c0 for %c4] : i32 -> !stream.resource<external>{%c4}
} => !stream.timepoint
%1 = stream.timepoint.await %0 => %result : !stream.resource<external>{%c4}
%2 = stream.tensor.export %1 : tensor<f32> in !stream.resource<external>{%c4} -> tensor<f32>
check.expect_almost_eq(%2, %2) : tensor<f32>
%result_0, %result_timepoint_1 = stream.resource.alloca uninitialized : !stream.resource<external>{%c4} => !stream.timepoint
%3 = stream.cmd.execute await(%result_timepoint_1) => with(%result_0 as %arg0: !stream.resource<external>{%c4}) {
stream.cmd.fill %c1090519040_i32, %arg0[%c0 for %c4] : i32 -> !stream.resource<external>{%c4}
} => !stream.timepoint
%4 = stream.timepoint.await %3 => %result_0 : !stream.resource<external>{%c4}
%5 = stream.tensor.export %4 : tensor<f32> in !stream.resource<external>{%c4} -> tensor<f32>
check.expect_almost_eq(%5, %5) : tensor<f32>
util.return
}
} |
|
I'm pretty sure the compile command I've put above is the one I saw printed in the CI log, but I can't find that log anymore as I've pushed an update to the PR where I saw it |
Carries same 2 reverts as previous integrates #22200 and #22214 Change in IREE for change some TOSA pass logic: llvm/llvm-project#153771 Change in IREE for deprecated LLVM Triple API: llvm/llvm-project#162186 Currently includes patch #22241 which is a pure IREE fix Increases golden times for 2 models (<5%): `assert 10.864054075338773 <= 10.5` Noticed this potential flake on windows at some point: lit test ksplitmatmul_basic Follow-up: understand #22241 (why can we not just assert it is not null?) --------- Signed-off-by: James Newling <[email protected]>
|
Closing, preference is for #22244 |
Carries same 2 reverts as previous integrates iree-org#22200 and iree-org#22214 Change in IREE for change some TOSA pass logic: llvm/llvm-project#153771 Change in IREE for deprecated LLVM Triple API: llvm/llvm-project#162186 Currently includes patch iree-org#22241 which is a pure IREE fix Increases golden times for 2 models (<5%): `assert 10.864054075338773 <= 10.5` Noticed this potential flake on windows at some point: lit test ksplitmatmul_basic Follow-up: understand iree-org#22241 (why can we not just assert it is not null?) --------- Signed-off-by: James Newling <[email protected]> Signed-off-by: Philipp <[email protected]>
Carries same 2 reverts as previous integrates iree-org/iree#22200 and iree-org/iree#22214 Change in IREE for change some TOSA pass logic: llvm/llvm-project#153771 Change in IREE for deprecated LLVM Triple API: llvm/llvm-project#162186 Currently includes patch iree-org/iree#22241 which is a pure IREE fix Increases golden times for 2 models (<5%): `assert 10.864054075338773 <= 10.5` Noticed this potential flake on windows at some point: lit test ksplitmatmul_basic Follow-up: understand iree-org/iree#22241 (why can we not just assert it is not null?) --------- Signed-off-by: James Newling <[email protected]>
Compilation:
Assertion failure:
For some reason this is only after an LLVM bump + 2 other fixes (see #22234).