- User Since
- Feb 24 2016, 11:36 AM (242 w, 5 d)
Updated based on review comments.
Fri, Oct 16
Thu, Oct 15
Tue, Oct 6
Mon, Oct 5
Missed one __r for normf.
Add missing __ prefix inside of function variables. Fix C++ and C-only macros, so that we don't use bool or the same function names in C-only. Also, use macros to define isfinite, isinf, isnan, and signbit to align with cmath !__cplusplus.
This is a pre-requisite for the next patch to add OpenMP w/ AMDGCN support.
Ran clang-format on this patch, and also verified on full stack (ROCm, Libraries, and Frameworks).
Fri, Oct 2
Mon, Sep 28
Could you please add full context? Mostly looks good to me.
Tue, Sep 22
Sep 17 2020
Adding Diag when mixing device and host paths with -emit-pch. Currently, we don't support this path, since the generated pch will be a clang_offload_bundle (supporting that will require that the ASTReader understand clang offload bundles and that is outside the scope of this patch). Added tests to check error Diag is reported when running both paths with -emit-pch.
Added a C++ header to .pch file test.
Sep 16 2020
Added a test checking for error when -o option is used for multi-device -emit-pch run.
Having -emit-pch in the clang driver is useful because it doesn't require users to specify standard C++ include paths, clang include paths, and CUDA/HIP wrapper headers needed by CC1. That is error prone for the user.
Also, this device compilation is not niche, it is needed for nvrtc/hiprtc and hip applications can perform device-only compilations at either compile-time or run-time.
Updated the tests to use --cuda-host-only or --cuda-device-only options when using -emit-pch. Added more tests for compilation when using -include-pch. Also, added a negative test when using different GPU variant PCH during compilation.
I believe one use-case for PCH is for common include headers such as hip_runtime.h which is being re-used in many application source files. To improve the performance, we can pre-compile the header and re-use it during online compilation.
Sep 10 2020
Fixed to use TY_HIPHeader instead of changing the phases in TY_HIP.
Looking into the C/C++ tests.
Sep 8 2020
LGTM, looks like its already in clang/include/clang/Basic/Cuda.h and clang/lib/Basic/Targets/AMDGPU.cpp.
Aug 24 2020
Aug 6 2020
Updated the NOFLAG_NOT to accept any prefix.
Jul 7 2020
Jul 6 2020
Jul 3 2020
Jun 29 2020
Hi Jon, I may have committed this in combination with D81963 as the parent commit. It should have waited for a LGTM from someone outside our org.
Jun 25 2020
Jun 22 2020
Hi, I am getting a compiler error due to this patch. I am using cmake command: cmake ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;lld;clang-tools-extra" -DLLVM_TARGETS_TO_BUILD="AMDGPU;X86" -DLLVM_ENABLE_ASSERTIONS=1 . Could you please take a look?
@yaxunl - I am looking into the bundler issue. Is it okay to commit these current patches first, while we look into that bug? This patch should only affect HIP.
Jun 17 2020
Jun 16 2020
Removed the HIP Linker changes, that will come in a different patch. Keeping this change more general.
I've updated this patch so it doesn't have dependence on the static-lib patch.
Rebased on master, and added --emit-static-lib to hip-link-save-temps.hip testcase.
Jun 15 2020
Thank you for this patch, it looks good, and I have a few minor questions/comments.
LGTM, thank you for splitting the cuda-phases from hip-phases, and opening up a follow-up patch for RDC case.
Jun 3 2020
Jun 2 2020
Thank you for the tips, I've updated the code with Driver::ShouldEmitStaticLibrary function, and renamed to flag to a more general name, --emit-static-lib.
Jun 1 2020
ping - is this refactored version ready for submission? Thanks.
May 28 2020
I've refactored the code, and removed the AddHIPLinkerScript function, which can be replaced by these MC directives approach.
May 27 2020
May 26 2020
Thank you for your reviews, I've cleaned up the patch, and reduced the size of the StaticLib::ConstructJob.
May 21 2020
May 13 2020
May 12 2020
Maybe Aaron can add a lit test for using this option for C language.
Apr 24 2020
Changed to use llvm-ar tool.
Apr 23 2020
Adding a new diff with full context.
Feb 27 2020
Jan 22 2020
Dec 3 2019
Oct 9 2019
Oct 3 2019
LGTM - Thanks for generalizing
Oct 2 2019
LGTM thank you
Oct 1 2019
Jul 10 2019
Apr 10 2019
Apr 3 2019
Thank you, I submitted a clang-formatted fix for the if statement.
Apr 2 2019
Hi Artem, I had just committed the change. IS this change OK or should I revert it?
Mar 15 2019
Mar 13 2019
Hi Matt, that solution will need refactoring and testing. Currently, HIP-Clang is following the same link flow as HCC
Feb 19 2019
This was submitted again with the fix. Thanks!
Feb 13 2019
Feb 12 2019
Yes, but it will need this PR: https://github.com/ROCm-Developer-Tools/HIP/pull/910
So that the default is set.