yaxunl added a comment.
Hi Tony,
I have already updated with a full diff. Please take a look. Thanks.
https://reviews.llvm.org/D31210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
t-tye requested changes to this revision.
t-tye added a comment.
This revision now requires changes to proceed.
Also please upload as full diff.
Comment at: lib/Basic/Targets.cpp:2026-2069
+ struct AddrSpace {
+unsigned Generic, Global, Local, Constant, Private;
+bool
yaxunl updated this revision to Diff 92977.
yaxunl added a comment.
Revised by Tony's suggestions.
https://reviews.llvm.org/D31210
Files:
lib/Basic/Targets.cpp
test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
Index: test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
===
yaxunl added a comment.
Ping! Any further issues? We need this in to move on with Clang codegen for the
new address space mapping.
Thanks.
https://reviews.llvm.org/D31210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.or
yaxunl updated this revision to Diff 92717.
yaxunl edited the summary of this revision.
yaxunl added a comment.
Rename the triple and variable names.
https://reviews.llvm.org/D31210
Files:
lib/Basic/Targets.cpp
test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
Index: test/CodeGenOpenCL/amdgpu-env-am
yaxunl added a comment.
In https://reviews.llvm.org/D31210#707842, @yaxunl wrote:
> In https://reviews.llvm.org/D31210#707832, @rampitec wrote:
>
> > I also do not exactly like names "old" and "new". This implies we are going
> > to switch to "new" permanently and doing transition. That is not c
rampitec added a comment.
In https://reviews.llvm.org/D31210#707842, @yaxunl wrote:
> In https://reviews.llvm.org/D31210#707832, @rampitec wrote:
>
> > I also do not exactly like names "old" and "new". This implies we are going
> > to switch to "new" permanently and doing transition. That is not
yaxunl added a comment.
In https://reviews.llvm.org/D31210#707832, @rampitec wrote:
> I also do not exactly like names "old" and "new". This implies we are going
> to switch to "new" permanently and doing transition. That is not clear yet,
> however.
How about we call the old address space ma
yaxunl added inline comments.
Comment at: lib/Basic/Targets.cpp:2029-2040
+ if (UseNew) {
+Generic = 0;
+Global= 1;
+Local = 3;
+Constant = 4;
+Private = 5;
+ } else {
tstellar wrote:
> What are these
rampitec added a comment.
I also do not exactly like names "old" and "new". This implies we are going to
switch to "new" permanently and doing transition. That is not clear yet,
however.
https://reviews.llvm.org/D31210
___
cfe-commits mailing list
tstellar added inline comments.
Comment at: lib/Basic/Targets.cpp:2029-2040
+ if (UseNew) {
+Generic = 0;
+Global= 1;
+Local = 3;
+Constant = 4;
+Private = 5;
+ } else {
What are these values used for
yaxunl updated this revision to Diff 92674.
yaxunl edited the summary of this revision.
https://reviews.llvm.org/D31210
Files:
lib/Basic/Targets.cpp
test/CodeGenOpenCL/amdgpu-env-amdnas.cl
Index: test/CodeGenOpenCL/amdgpu-env-amdnas.cl
yaxunl updated this revision to Diff 92587.
yaxunl added a comment.
Fix getDWARFAddressSpace.
https://reviews.llvm.org/D31210
Files:
include/clang/Driver/Options.td
lib/Basic/Targets.cpp
test/CodeGenOpenCL/amdgpu-new-addr.cl
Index: test/CodeGenOpenCL/amdgpu-new-addr.cl
==
yaxunl added a comment.
In https://reviews.llvm.org/D31210#707014, @scchan wrote:
> In https://reviews.llvm.org/D31210#706955, @yaxunl wrote:
>
> > In https://reviews.llvm.org/D31210#706890, @kzhuravl wrote:
> >
> > > In https://reviews.llvm.org/D31210#706880, @rampitec wrote:
> > >
> > > > I'm c
scchan added a comment.
In https://reviews.llvm.org/D31210#706955, @yaxunl wrote:
> In https://reviews.llvm.org/D31210#706890, @kzhuravl wrote:
>
> > In https://reviews.llvm.org/D31210#706880, @rampitec wrote:
> >
> > > I'm concerned about the default address space to be 64 bit. It would move
>
yaxunl added a comment.
In https://reviews.llvm.org/D31210#706890, @kzhuravl wrote:
> In https://reviews.llvm.org/D31210#706880, @rampitec wrote:
>
> > I'm concerned about the default address space to be 64 bit. It would move
> > alloca into generic address space effectively making private addre
kzhuravl added a comment.
In https://reviews.llvm.org/D31210#706880, @rampitec wrote:
> I'm concerned about the default address space to be 64 bit. It would move
> alloca into generic address space effectively making private address to be 64
> bit.
> This may have very undesirable performance
rampitec added a comment.
I'm concerned about the default address space to be 64 bit. It would move
alloca into generic address space effectively making private address to be 64
bit.
This may have very undesirable performance implications, like address
arithmetic can become expensive 64 bit and
kzhuravl added a comment.
getDWARFAddressSpace also needs to be updated.
https://reviews.llvm.org/D31210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaxunl created this revision.
Herald added subscribers: Anastasia, tpr, dstuttard, nhaehnle, wdng, kzhuravl.
A new target feature new-addr is added. When it is on, AMDGPU uses new address
space mapping where generic address space is 0 and private address space is 5.
The data layout is also chang
20 matches
Mail list logo
