User Details
- User Since
- Apr 23 2020, 6:41 PM (51 w, 4 d)
Today
Review comments addressed.
Reopening this. This version is supposed to fix the buildbot failures on PPC machines.
Since I don't have PPC machine I am not sure if this will work. But the logic
followed here is motivated from Clang :: Driver/program-path-priority.c, so hopefully
it will pass the CI.
Fri, Apr 16
I've reverted this from main for now as there seems to be issue with executing test script on some CI machines.
Thu, Apr 15
Wed, Apr 14
Rebase and review comments
Fri, Apr 9
Fix permissions
Added tests for the failing cases
Thu, Apr 8
- Addressed review comments.
- Addressed review comments
- Added LIT test case
Wed, Apr 7
Addressed review comments.
Tue, Apr 6
Working on tests.
Wed, Mar 31
Mon, Mar 29
The general problem seems bit more involved. I am not that familiar with how other architectures/systems handle the library/include path. Simplest solution that I can propose right now is to generalise my revision to other architectures for header lookup and similarly for library lookup in case of openmp. If there is better solution available please let me know I will be happy to implement it.
Fri, Mar 26
I was not aware that it was a general problem. I will check if I could get a general solution.
Mon, Mar 22
Fix comments.
- Revert the unsigned case to use Masks.
- Simplified the logic for widenScalar
Mar 19 2021
Removed assert. Now WideTy overflow check is only done when WideTy is not sufficient enough.
Added test case for s24 to verify the logic.
Mar 18 2021
Use SHRL for unsigned case.
Mar 17 2021
Ping!
Mar 15 2021
Mar 9 2021
Rebase & Review comments
Feb 25 2021
Add extra llc step to produce assembly in the linker.
So, neither emit-llvm-bc or emit-llvm work well with save-temps. Therefore, I feel the current approach is still valid. This does not impact nvptx or any other target in any way. And I don't see how.
Feb 24 2021
Feb 23 2021
Here's a bit of background,
OffloadingPrefix was not getting properly set in the dependent actions of OffloadWrapperJobAction (which are backend [11] and assemble [12]). Since backend [11] and assemble [12] host-wrapper actions have same logic to the other host actions (3 & 4), those will overwrite the previous generated files from host-only actions.
Feb 18 2021
Looks good to me. Thanks!
Feb 17 2021
Fixed the assert.
Feb 16 2021
It is because of how addClangTargetOptions is invoked. In case of save-temps, it is being invoked for all the actions resulting in target cc1 call. That's why all these invocations have -emit-llvm-bc. I guess we need Action as an argument to addClangTargetOptions.
emit-llvm-bc does not correctly solve the problem. It works because [input, compile, assemble, backend] actions collapse to a single action by driver. This single command handles emit-llvm-bc properly. But when save-temps is specified, this collapsing does not happen which messes up command line flags of the jobs and hence the output, for e.g., preprocessor command also has -emit-llvm-bc.
This does fixes the save-temps but only when -o is not specified. If -o is specified (along with -save-temps), the name of host object file and host-wrapper object file (second last phase) is same, which fails the linker. This does not seem to be related to this patch.
Feb 15 2021
Addressed review comments.
Can you use -check-prefixes=GCN,GFX8 and GCN,GFX9 so that update_mir_test_checks will common up the identical ones?
It does not work. Script warns as WARNING: Ignoring common prefixes: {'GCN'}: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-umulo.mir
Addressed review comments.
Feb 11 2021
Feb 10 2021
I have removed libomptarget-device-bc-path and have added amdgcn one. For diagnostic,
instead of having one per architecture, I have used the same and added second
parameter to err_drv_omp_offload_target_missingbcruntime for having arch specifc message.
Feb 9 2021
LGTM. Thanks!
LGTM, thanks for fixing it.
Feb 8 2021
- Added check for nogpulib
- Fixed diagnostic message
Addressed review comments.
Missed some changes,
- Fix openmp-offload.c test failure
- Fix amdgpu-openmp-toolchain.c test failure
Ping!
Feb 2 2021
- Use 0 for default -O option
- Rename addOptLevelArgs to addLLCOptArg
After addressing the review comments, I have internally verified changes on few simple test programs. They seem to be working fine.
Addressed review comments.
- Combined the toolchain creation logic for nvptx and amdgcn
- Replaced -Xopenmp-target with -emit-llvm-bc inside AMDGPUOpenMP.cpp
- Removed opt from pipeline
Feb 1 2021
- Scalarize the vectors first
- Using widened operation for smaller types
Ping!
Jan 28 2021
Hi, apologies for late reply as I got sidetracked to some other work.
Jan 27 2021
Jan 20 2021
- Moved common methods of HIP and OpenMP to base AMDGPUToolChain
- Removed unnecessary asserts
Fixed failing debian tests
Won't this just prevent us from building clang due to the missing cmake changes?
It compiles and builds fine, however, I wasn't actually aware such sanity checking being present. It turns out
the unknown files inside llvm/ will lead cmake to report error but such reporting will not happen inside clang. Maybe such checks
were not enabled inside clang. Anyways thanks for pointing out. I will keep that in mind in future.
Jan 19 2021
Fix clang-tidy error
Jan 14 2021
Moved ops close to ADDO
Jan 10 2021
Removed global-isel-abort=0
Jan 5 2021
Jan 4 2021
Jan 1 2021
Dec 20 2020
Update AMDGPU barrier intrinsic
Dec 7 2020
Ping!
Dec 3 2020
Looks good, thanks.