-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[test] Enable cfg_optimization in all tests #2106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... Thanks for investigating this :-) Maybe a recent change somewhere in the type system fixes the CFG optimizations. Btw, could you add a check of CompileConfig::cfg_optimization
in full_simplify
, to fix the behavior of that boolean flag? Feel free to merge after the flag is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! It seems that skipping cfg_optimization
pass is not necessary now. I added this flag when I was not very familiar with Taichi compilation system. At that time, I tried to set and verify data in one single kernel like:
cit = ti.type_factory_.get_custom_int_type(16, True)
x = ti.field(dtype=cit)
ti.root._bit_struct(32).place(x)
@ti.kernel
def test_custom_int_type(data: ti.i32):
x = data
assert x == data
And I found that after the pass of cfg_optimization
, the assignment statement would always be skipped. So, to make sure the assignment could be executed, I turn off this optimization. Now, almost all of our test cases do the setting and verifying in two separate kernels, which is a better way, so it is not necessary to keep cfg_optimization
flag false.
Thank you so much for investigating this!
I see! For this kernel @ti.kernel
def test_custom_int_type(data: ti.i32):
x[None] = data
assert x[None] == data , the IR is
. When
So IIUC |
Yes you are right, that is the exact reason why I turn this optimization pass off. Thanks for the very detailed IR analysis! |
Related issue = #1905
All the tests passed on my end. I'm not sure in which way will the control-flow graph optimizations incompatible with the new type system?
BTW, in
compile_to_offloads.cpp
, this part may be redundant and may not perform the check it intends to:taichi/taichi/transforms/compile_to_offloads.cpp
Lines 105 to 109 in e251ca7
-- this is because
cfg_optimization
is already called infull_simplify
without checking compile config here:taichi/taichi/transforms/simplify.cpp
Lines 616 to 618 in e251ca7
[Click here for the format server]