[SYCL][NATIVECPU][PERF] Avoid generating uncalled kernel subhandlers #17353
Conversation
uwedolinsky
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
uwedolinsky
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| @@ -404,7 +404,8 @@ PreservedAnalyses PrepareSYCLNativeCPUPass::run(Module &M, | |||
| NewF->takeName(OldF); | |||
| OldF->replaceAllUsesWith(NewF); | |||
| OldF->eraseFromParent(); | |||
| NewKernels.push_back(NewF); | |||
| if (NewF->getName() != "") | |||
| NewKernels.push_back(NewF); | |||
There was a problem hiding this comment.
Since NewF takes its name from OldF, this would only apply to functions where originally OldF.getName() == "", right? If so, could we skip the whole cloneFunctionAndAddParam as well and just do if (OldF.getName() == "") continue; at the start? Or maybe even keep it out of OldKernels entirely?
There was a problem hiding this comment.
Cloning may not be needed in this case right now, but we will need it again when we don't need the thread_local, which is another optimization PR I'm working on. So could remove it now for this case but would need to put it back in later again.
There was a problem hiding this comment.
When we don't need the thread_local, can we not just continue using OldF and thereby still avoid the clone? But maybe I am misunderstanding what you mean.
There was a problem hiding this comment.
Are we sure that NewF can not have a name anyway? In what situation does that happen?
There was a problem hiding this comment.
When we don't need the thread_local, can we not just continue using
OldFand thereby still avoid the clone? But maybe I am misunderstanding what you mean.
This PR just intends to avoid generating uncalled subhandlers in the smallest self-contained way. I guess we could add more optimizations related to kernel cloning, but we would probably have to change scope and description.
There was a problem hiding this comment.
Are we sure that NewF can not have a name anyway? In what situation does that happen?
It does have a name when it wasn't replaced - see the old comment in the code.
uwedolinsky
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
uwedolinsky
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
uwedolinsky
changed the title
This PR prevents the generation of NativeCPU kernel subhandlers that cannot be called.
Also reduces number of kernel lookups during compilation.