Hahnfeld added a comment.
Let's start with the obvious points:
1. The latest patch clearly wasn't run through `clang-format`.
2. It only has some 90 lines of context which makes it look like you deleted
quite some code when browsing through the history.
3. This patch is still large, did you at l
yaxunl updated this revision to Diff 134107.
yaxunl added a comment.
Update with Greg's change.
https://reviews.llvm.org/D42800
Files:
include/clang/Basic/Cuda.h
include/clang/Driver/ToolChain.h
lib/Basic/Cuda.cpp
lib/Basic/Targets/AMDGPU.cpp
lib/Basic/Targets/AMDGPU.h
lib/Basic/Tar
gregrodgers added a comment.
Here my replys to the inline comments. Everything should be fixed in the next
revision.
Comment at: include/clang/Basic/Cuda.h:79
COMPUTE_72,
+ COMPUTE_GCN,
};
t-tye wrote:
> Suggest using amdgcn which matches the architectu
yaxunl added inline comments.
Comment at: lib/Basic/Targets/AMDGPU.h:85
return TT.getEnvironmentName() == "amdgiz" ||
+ TT.getOS() == llvm::Triple::CUDA ||
TT.getEnvironmentName() == "amdgizcl";
t-tye wrote:
> We still want to use the a
t-tye added inline comments.
Comment at: include/clang/Basic/Cuda.h:49-57
+ GFX700,
+ GFX701,
+ GFX800,
+ GFX801,
+ GFX802,
+ GFX803,
+ GFX810,
Should complete list of processors for the amdgcn architecture be included? See
https://llvm.org/docs/AMDGPUUsa
gregrodgers added a comment.
Sorry, all my great inline comments got lost somehow. I am a newbie to
Phabricator. I will try to reconstruct my comments.
https://reviews.llvm.org/D42800
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
gregrodgers requested changes to this revision.
gregrodgers added a comment.
This revision now requires changes to proceed.
Thanks to everyone for the reviews. I hope I replied to all inline comments.
Since I sent this to Sam to post, we discovered a major shortcoming. As tra
points out, the
tra added a comment.
I don't have enough knowledge about compute on AMD's GPU and would appreciate
if you could share your thoughts on how you think CUDA on AMD should work. Is
there a good document describing how compute currently works (how do I launch a
kernel using rough equivalent of nvidi
arsenm added inline comments.
Comment at: lib/Basic/Targets/AMDGPU.cpp:437
+ case CudaArch::UNKNOWN:
+assert(false && "No GPU arch when compiling CUDA device code.");
+return "";
llvm_unreachable
Comment at: lib/Driver/Tool
yaxunl added a comment.
In https://reviews.llvm.org/D42800#994955, @Hahnfeld wrote:
> Only commenting on parts that I'm a bit familiar with. In general, does it
> make sense to split this patch, are there different "stages" of support? Like
> 1) being able to compile an empty file, 2) generate
Hahnfeld added a comment.
Only commenting on parts that I'm a bit familiar with. In general, does it make
sense to split this patch, are there different "stages" of support? Like 1)
being able to compile an empty file, 2) generate optimized code, 3) allow using
math functions?
==
yaxunl created this revision.
yaxunl added reviewers: gregrodgers, jlebar, b-sumner, t-tye, arsenm.
Herald added subscribers: tpr, dstuttard, nhaehnle, wdng, kzhuravl, jholewinski.
Currently CUDA toolchain only supports nvptx.
This patch will let CUDA toolchain support amdgpu target. It can also
12 matches
Mail list logo
