[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. In D127142#3601810 , @yaxunl wrote: > In D127142#3600809 , @MaskRay wrote: > >> Magically deciding a default value for --unwindlib or --rtlib is not nice. >> You may emit a warning if the

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl added a comment. In D127142#3600809 , @MaskRay wrote: > Magically deciding a default value for --unwindlib or --rtlib is not nice. > You may emit a warning if the selected default happens to be incompatible > with HIP. We build clang not just fo

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. Magically deciding a default value for --unwindlib or --rtlib is not nice. You may emit a warning if the selected default happens to be incompatible with HIP. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127142/new/ https://reviews.llvm.org/D127142 __

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl updated this revision to Diff 438709. yaxunl added a comment. add -unwindlib=libgcc by default for --hip-link since -rtlib=compiler-rt needs it CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127142/new/ https://reviews.llvm.org/D127142 Files: clang/lib/Driver/ToolChain.cpp c

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. In D127142#3592207 , @yaxunl wrote: > In D127142#3590874 , @yaxunl wrote: > >> In D127142#3571260 , @MaskRay >> wrote: >> >>> In D127142#3570290

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl added a comment. In D127142#3590874 , @yaxunl wrote: > In D127142#3571260 , @MaskRay wrote: > >> In D127142#3570290 , @yaxunl wrote: >> >>> If I use --rtlib=compile

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl added a comment. In D127142#3571260 , @MaskRay wrote: > In D127142#3570290 , @yaxunl wrote: > >> If I use --rtlib=compiler-rt, does that also requires --unwindlib=unwindlib ? > > No. --unwindlib=libunwind r

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl updated this revision to Diff 437587. yaxunl added a comment. use compiler-rt as runtime lib by default for --hip-link CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127142/new/ https://reviews.llvm.org/D127142 Files: clang/lib/Driver/ToolChain.cpp clang/test/Driver/hip-runti

[PATCH] D127142: [HIP] Link with clang_rt.builtins

tra added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:684-690 CmdArgs.append( {Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()), "-rpath", Args.MakeArgString(RocmInstallation.getLibPath())}); CmdArgs.push_back("-lam

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:684-690 CmdArgs.append( {Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()), "-rpath", Args.MakeArgString(RocmInstallation.getLibPath())}); CmdArgs.push_back("

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. In D127142#3570290 , @yaxunl wrote: > In D127142#3568905 , @MaskRay wrote: > >>> These functions are not available in libgcc but in libclang_rt.builtins. >>> Therefore --hip-link needs to

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl added a comment. In D127142#3568905 , @MaskRay wrote: >> These functions are not available in libgcc but in libclang_rt.builtins. >> Therefore --hip-link needs to link with libclang_rt.builtins by default. > > I think this is problematic. > > The

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. > These functions are not available in libgcc but in libclang_rt.builtins. > Therefore --hip-link needs to link with libclang_rt.builtins by default. I think this is problematic. The current link sequence is `... "-lamdhip64" "/tmp/Debug/lib/clang/15.0.0/lib/linux/libc

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl updated this revision to Diff 434807. yaxunl marked 3 inline comments as done. yaxunl edited the summary of this revision. yaxunl added a comment. use getCompilerRT to get compiler-rt lib path CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127142/new/ https://reviews.llvm.org/D127

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl marked 3 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:485 "amdhip64.lib"}); + CmdArgs.push_back(Args.MakeArgString("clang_rt.builtins-" + + getTriple().getA

[PATCH] D127142: [HIP] Link with clang_rt.builtins

tra added a reviewer: MaskRay. tra added a comment. Herald added a subscriber: StephenFan. Looks OK syntax-wise. Library path should probably be fixed, though it appears to be a somewhat separate issue and could be done in its own patch, if the required change is not trivial. ==

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:485 "amdhip64.lib"}); + CmdArgs.push_back(Args.MakeArgString("clang_rt.builtins-" + + getTriple().getArchName() + ".lib")); N

[PATCH] D127142: [HIP] Link with clang_rt.builtins

yaxunl created this revision. yaxunl added a reviewer: tra. Herald added a project: All. yaxunl requested review of this revision. Herald added a subscriber: MaskRay. HIP supports _Float16 by default in host programs, which may cause calls of conversion functions for _Float16 emitted e.g. `__trunc