-
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
[ir] Make statements final, and change IRNode::as
to use static_cast
#7896
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docsite-preview canceled.
|
for more information, see https://pre-commit.ci
/benchmark |
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.
Unless there's significant performance gain, I would suggest that we keep the type check TI_ASSERT(is<T>());
.
static_cast
to Derived class pointers doesn't perform any check and is easy to cause undefined behavior if not used with caution.
Benchmark ReportBaseline:
|
perf gain is minimal probably because modern CPUs getting too good at prediction. I think a potential way to do this is to use system assert statement instead of |
Agreed! |
Diff of 0% seems pretty weird. Is there anything wrong with our baseline? @feisuzhu |
In theory and in small scale experiments with different compilers, this does result in less calls to
dynamic_cast
and potentially cheaperdynamic_cast
& virtual functions. However on Windows the benefits seem to be within run-to-run variance at least on my machine.Walkthrough
🤖 Generated by Copilot at 310a4e3
dynamic_cast
withstatic_cast
forIRNode::as
method to improve performance (link)final
keyword to all statement classes intaichi/ir/statements.h
to prevent unintended inheritance (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)