Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I'm not familiar with the details of toolchain config, but added some general comments.
clang/lib/Driver/ToolChains/CommonArgs.cpp | ||
---|---|---|
1404 | Collapse this into if isohos && triple.getarch is.... | |
1694 | Please add a comment explaining this. Even if it is just "OHOS is only compatible with libunwind". At least then we know it's nothing more mysterious. | |
clang/lib/Driver/ToolChains/Gnu.cpp | ||
645–647 | And musl is what OHOS uses? Please add that to the comment if so "for musl, which OHOS uses...". | |
2319 | No riscv related changes needed in this file? | |
clang/lib/Driver/ToolChains/OHOS.cpp | ||
165 | Keep FIXMEs that refer to stuff outside the project downstream please :) | |
373 | Maybe this helps? https://github.com/llvm/llvm-project/blob/e00c73c856a325008afead10cfb3e9d0fc4a1e41/clang/lib/Driver/ToolChains/Linux.cpp#L235 (no idea myself) | |
clang/test/Driver/ohos.c | ||
241 | This one is checking that we do not link to a pthread library, because when using musl, it already provides one? Just checking you're not accepting the option but doing the opposite here. Worth adding a comment to explain the reasoning. |
clang/test/Driver/ohos.c | ||
---|---|---|
241 | Yes, musl implements pthread api, so we do not need to link with libpthread |
Good morning,
I believe the two tests added in this patch, clang::ohos.c and clang::ohos.cpp are failing on the following buildbot:
https://lab.llvm.org/buildbot/#/builders/216/builds/18323
is anyone able to take a look?
thanks,
Tom W
Also failing on our Windows on Arm bot: https://lab.llvm.org/buildbot/#/builders/65/builds/8607
I think some lines need to account for Windows slashes, {{/|\\\\}} should do that.
This is also causing a breakage on the Linux and Mac Fuchsia toolchain builds:
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8786633328903858561/overview
It looks like these tests don't pass when LLVM_ENABLE_LINKER_BUILD_ID is set; I've issued a revert until this can be fixed to unblock the Fuchsia toolchain.
clang/lib/Driver/ToolChains/OHOS.h | ||
---|---|---|
39 | Why is useRelaxRelocations false? |
clang/lib/Driver/ToolChains/OHOS.h | ||
---|---|---|
39 | Thank you for mentioning. I would say that there is no precise reason for it. And it will be changed to true in the nearest future. |
Collapse this into if isohos && triple.getarch is....