-
Notifications
You must be signed in to change notification settings - Fork 787
[sycl-post-link] Enable ESIMD-specific lowering #3195
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
This is the first patch in the effort of moving ESIMD-specific lowering passes from clang FE (BackendUtils.cpp) to the sycl-post-link tool after SYCL-ESIMD splitting.
A couple of comments explaining certain implementation details:
This patch doesn't have an immediate impact. In a later patch, I will add the corresponding code in the clang driver. |
// When ESIMD code was separated from the regular SYCL code, | ||
// we can safely process ESIMD part. |
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.
This is probably an architecture question/concern: why can't we leave a single optimization pipeline which is executed before sycl-post-link
invocation?
I understand that IGC VC backend most likely expects LLVM IR in some particular form to do its job right, but can't it be improved? It seems a bit strange to me that we have to maintain two optimization pipelines in two different places. Moreover, I guess that the first pipeline before sycl-post-link
will still be executed to optimize regular non-ESIMD code, right?
Note: I'm okay if we have to launch a few more passes in order to launch some ESIMD
-specific ones, but I would like to have a single optimization pipeline (potentially customized compared to the upstream one) for general-purpose optimizations.
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.
@AlexeySachkov, yes, those are ESIMD-specific transformations that should be applied to ESIMD code only, otherwise, it will break regular SYCL code. So, it should be done after splitting. Or we should clone the code that is shared between ESIMD and regular SYCL kernels and then do the ESIMD lowering. Splitting first and then transform, eliminates the need to clone shared code, so I guess this is definitely an advantage.
Yes, we have general-purposes clean-up passes after ESIMD lowering. So, yes, we have 2 sets of general-purpose passes running on ESIMD code. I believe we have to have the second set after ESIMD lowering. @cmc-rep can confirm.
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.
those are ESIMD-specific transformations that should be applied to ESIMD code only, otherwise, it will break regular SYCL code.
I'm not against ESIMD-specific transformation, I'm concerned about additional optimization passes we launch in sycl-post-link
Yes, we have general-purposes clean-up passes after ESIMD lowering. So, yes, we have 2 sets of general-purpose passes running on ESIMD code. I believe we have to have the second set after ESIMD lowering. @cmc-rep can confirm.
To me it seems better if those post-processing is moved into vc backend as pre-processing passes, because this is a thing specific to a single backend, not even to a language constructs.
static cl::opt<bool> | ||
OptLevelO0("O0", cl::desc("Optimization level 0. Similar to clang -O0"), | ||
cl::cat(PostLinkCat)); | ||
|
||
static cl::opt<bool> | ||
OptLevelO1("O1", cl::desc("Optimization level 1. Similar to clang -O1"), | ||
cl::cat(PostLinkCat)); | ||
|
||
static cl::opt<bool> | ||
OptLevelO2("O2", cl::desc("Optimization level 2. Similar to clang -O2"), | ||
cl::cat(PostLinkCat)); | ||
|
||
static cl::opt<bool> OptLevelOs( | ||
"Os", | ||
cl::desc( | ||
"Like -O2 with extra optimizations for size. Similar to clang -Os"), | ||
cl::cat(PostLinkCat)); | ||
|
||
static cl::opt<bool> OptLevelOz( | ||
"Oz", | ||
cl::desc("Like -Os but reduces code size further. Similar to clang -Oz"), | ||
cl::cat(PostLinkCat)); | ||
|
||
static cl::opt<bool> | ||
OptLevelO3("O3", cl::desc("Optimization level 3. Similar to clang -O3"), | ||
cl::cat(PostLinkCat)); |
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.
How these are expected to be used? Will they be propagated to the tool based on the same options passed to clang driver?
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.
Yes, I will submit the patch for that later and put the link here.
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.
Here is the driver portion that will pass the optimization level to sycl-post-link: #3207.
Co-authored-by: kbobrovs <[email protected]>
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.
LGTM
@AlexeySachkov please review/approve. |
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.
I'm still concerned about having additional optimization pipeline in sycl-post-link
, but this concern shouldn't block the commit
@DenisBakhvalov, this patch breaks the build with shared libraries. https://github.com/intel/llvm/runs/1903069847?check_suite_focus=true |
@bader, Taking a look... |
Opened #3221. |
This is the first patch in the effort of moving ESIMD-specific lowering passes from clang FE (BackendUtils.cpp) to the sycl-post-link tool after SYCL-ESIMD splitting. Co-authored-by: kbobrovs <[email protected]>
This is one of the patches for moving ESIMD passes from clang FE (BackendUtils.cpp) to sycl-post-link. This patch is a continuation for #3195 and it passes the optimization level to the sycl-post-link.
This is the first patch in the effort of moving ESIMD-specific
lowering passes from clang FE (BackendUtils.cpp) to the
sycl-post-link tool after SYCL-ESIMD splitting.