-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[BugFix] Fix Crash Cases Caused by "__tvm_meta__ = None" #16634
Conversation
Run these test cases in local with this patch, below 3 test cases still will be failed because of TIR structure isn't equal, @Hzfengsy @tqchen can you help to see them? Thanks. tir-transform: |
The reason of why the crashed test cases won't be caught by CI is the pytest exist code is 5, it means no tests is collected, in file "tests/scripts/setup-pytest-env.sh" we haven't treat the exist code of 5 as error. I found another test item "tests/python/usmp" have the same exist code 5, because now there isn't the directory "tests/python/usmp", so pytest can't collect any test cases, I think "usmp" is a typo from #16110, so I delete it. |
cc @tqchen |
I had this issue come up in #16569. I think these tests probably were not being executed before. I am also pretty sure it's safe to just remove those lines, as In my PR, I added a check in the parser to make sure that |
I am also having this error in my PR 😂 For |
I've figured out, thanks to @Lunderberg's comments in my PR, that the failure in IterVar iter_var(Range(nullptr), Var("iter", dtype), IterVarType::kThreadIndex, thread); to IterVar iter_var(Range(nullptr), Var("iter", DataType::Int(32)), IterVarType::kThreadIndex, thread); (what it was before the merge commit that Eric pointed out), the test will pass. However, I'm not sure we want to manually fix the data type like that. This probably means that we should update the index narrowing pass to deal with the thread index itervar accordingly. |
I did a git bisect for |
Bisect for |
Fix for the bool IsScheduledOnGPU(const BaseFunc& func) {
// the target from context.
tvm::Target target = tvm::Target::Current();
// the Target in kTarget attribute of PrimFunc
Optional<tvm::Target> func_target = func->attrs.GetAttr<tvm::Target>(tvm::attr::kTarget);
if (func_target.defined()) {
target = func_target.value();
}
if (target.defined()) {
int dev_type = target->GetTargetDeviceType();
// Change is here. I don't know if web GPU is quite correct
if (!(dev_type == kDLCUDA || dev_type == kDLMetal || dev_type == kDLROCM ||
dev_type == kDLWebGPU)) {
return false;
}
}
return true;
} Note that this issue was mentioned in the code review for the PR. I will include this fix in my PR. |
Should we instead check whether the target tags include |
I'm not sure all of them actually contain the string "gpu." I will double check that. Edit: Indeed, Metal is simply called "Metal" |
These bugs should all be fixed now that #16569 was merged. I recommend you close unless there's a change in here that wasn't covered there. |
#16770 The sage continues... |
Hi Steven, I'm little confused. Does these three cases been fixed? I tried main with commit: 240326, b2204ae and update status as below. It seems still two cases failed:
|
Test with tvm.main.20240407.81a850693, 3 tests all passed. |
Fix #16633
The broken tests are below items of "tests/scripts/task_python_unittest.sh"
"tir-transform"
"tir-usmp"
The crashed cases:
tests/python/tir-transform/test_tir_transform_inject_rolling_buffer.py
tests/python/tir-usmp/test_tir_usmp_algo.py
tests/python/tir-usmp/test_tir_usmp_analysis_extract_bufferinfo.py
tests/python/tir-usmp/test_tir_usmp_transform_convert_pool_allocations_to_offsets.py
tests/python/tir-usmp/test_tir_usmp_utils.py
other failed cases:
tests/python/tir-transform/test_tir_transform_pointer_value_type_rewrite.py::TestRewriteToShuffle::test_compare
tests/python/tir-transform/test_tir_transform_force_narrow_index_to_i32.py::test_thread_axis2
tests/python/tir-transform/test_tir_transform_hoist_if.py::test_hoisting_block_scope_4
tests/python/tir-transform/test_transform_default_gpu_schedule.py::test_add_on_metal