[SYCL] Fixed an issue building shared-libs config#3301
Conversation
DenisBakhvalov
requested review from
elizabethandrews and
premanandrao
as code owners
DenisBakhvalov
force-pushed
the
dbakhval/fix-shared-libs
branch
from
78cb504 to
759f114
Compare
| clangCrossTU | ||
| clangFrontend | ||
| clangLex | ||
| clangSema |
There was a problem hiding this comment.
Why this change is needed? Is this dependency introduced by SYCL changes?
There was a problem hiding this comment.
Why this change is needed?
Some other failures were hidden by the one that I fixed with the change in llvm/lib/Passes/CMakeLists.txt. I.e. after I fixed one unresolved external problem there were 4 more.
Is this dependency introduced by SYCL changes?
Yes, I believe this failure was triggered by the recent pulldown to SYCL.
I'm not familiar with this part of FE, so I let FE people decide whether this dependency makes sense.
There was a problem hiding this comment.
This looks very suspicious. Community version doesn't have this dependency. https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CMakeLists.txt#L92
+@erichkeane, @AaronBallman, do you know the reason for clang's CodeGen to depend on Sema library?
There was a problem hiding this comment.
I'm going to merge this PR, but I'll open a separate issue to understand what's going on here.
There was a problem hiding this comment.
I think Clang's CodeGen library relies on Sema because of: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGDecl.cpp#L44 but I admit that this looks really suspicious to me. I could have sworn this is a layering violation (now Sema relies on CodeGen which relies on Sema). The patch that introduced the change was: llvm/llvm-project@0d61cd2 and it had all of 15 minutes of review before landing, and after landing someone pointed out the layering violation but no rollback happened. I've commented on it in community.
A recent spirv-val change requires that OpDecorateId IDs are well-ordered, which means that the decoration operand ID cannot be the same as the decoration target ID. See: KhronosGroup/SPIRV-Tools#6227 This leads to the failure: ``` error: line 6: Parameter <ID> '2[%uint_0]' must appear earlier in the binary than the target OpDecorateId %uint_0 UniformId %uint_0 ``` The fix is to use a different ID for the decoration operand and the decoration target. Original commit: KhronosGroup/SPIRV-LLVM-Translator@fc5873ee760c333
4 participants
No description provided.