This revision was automatically updated to reflect the committed changes.
Closed by commit rL299965: [OpenCL] Map default address space to alloca address
space (authored by yaxunl).
Changed prior to commit:
https://reviews.llvm.org/D31404?vs=94399=94855#toc
Repository:
rL LLVM
Anastasia accepted this revision.
Anastasia added a comment.
LGTM! Thanks!
https://reviews.llvm.org/D31404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaxunl marked 2 inline comments as done.
yaxunl added a comment.
Any further comments? Thanks.
https://reviews.llvm.org/D31404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.
Comment at: include/clang/Basic/AddressSpaces.h:32
+ // QualType represents private address space in OpenCL source code.
+ Default = 0,
+
Anastasia wrote:
> yaxunl wrote:
> > Anastasia
Anastasia added inline comments.
Comment at: include/clang/Basic/AddressSpaces.h:32
+ // QualType represents private address space in OpenCL source code.
+ Default = 0,
+
yaxunl wrote:
> Anastasia wrote:
> > The alloca AS is not taken from the target AS map
yaxunl updated this revision to Diff 94399.
yaxunl marked 5 inline comments as done.
yaxunl added a comment.
Revised by Anastasia's comments.
https://reviews.llvm.org/D31404
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Type.h
include/clang/Basic/AddressSpaces.h
yaxunl marked 11 inline comments as done.
yaxunl added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
Anastasia wrote:
> yaxunl wrote:
> > yaxunl wrote:
> > >
Anastasia added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
yaxunl wrote:
> yaxunl wrote:
> > Anastasia wrote:
> > > Anastasia wrote:
> > > > yaxunl wrote:
> >
yaxunl added a comment.
Ping! Any further questions? Thanks.
https://reviews.llvm.org/D31404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaxunl updated this revision to Diff 94102.
yaxunl marked 2 inline comments as done.
yaxunl added a comment.
Revised by Anastasia's comments.
Add lit test for `__attribute__((address_space(0)))` and default address space.
https://reviews.llvm.org/D31404
Files:
include/clang/AST/ASTContext.h
yaxunl marked 8 inline comments as done.
yaxunl added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
Anastasia wrote:
> Anastasia wrote:
> > yaxunl wrote:
> > >
Anastasia added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
Anastasia wrote:
> yaxunl wrote:
> > Anastasia wrote:
> > > So we couldn't use the LangAS::Count
Anastasia added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
yaxunl wrote:
> Anastasia wrote:
> > So we couldn't use the LangAS::Count instead?
> >
> > I have
t-tye accepted this revision.
t-tye added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D31404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
t-tye added inline comments.
Comment at: include/clang/AST/Type.h:339-340
+auto Addr = getAddressSpace();
+if (Addr == 0)
+ return 0;
+return Addr - LangAS::target_first;
Anastasia wrote:
> yaxunl wrote:
> > t-tye wrote:
> > > Since you mention
yaxunl marked 31 inline comments as done.
yaxunl added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
Anastasia wrote:
> So we couldn't use the LangAS::Count
t-tye added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:2055
+ << AllocType.getUnqualifiedType()
+ << AllocType.getQualifiers().getAddressSpacePrintValue();
else if (getLangOpts().ObjCAutoRefCount) {
Would suggest renaming
Anastasia added inline comments.
Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling ||
+ AS >= LangAS::target_first;
}
So we couldn't use the LangAS::Count instead?
I have the same comment in other places that use
yaxunl updated this revision to Diff 93870.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Revised by Tony's comments.
https://reviews.llvm.org/D31404
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Type.h
include/clang/Basic/AddressSpaces.h
t-tye added inline comments.
Comment at: include/clang/AST/Type.h:339-340
+auto Addr = getAddressSpace();
+if (Addr == 0)
+ return 0;
+return Addr - LangAS::target_first;
Since you mention this is only used for
yaxunl updated this revision to Diff 93820.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Update for the llvm IRBuilder API change for alloca. Removed data layout
argument.
Moved a lit test to SemaOpenCL.
https://reviews.llvm.org/D31404
Files:
yaxunl marked 3 inline comments as done.
yaxunl added inline comments.
Comment at: lib/AST/ASTContext.cpp:9553
+ // alloca.
+ if (AS == LangAS::Default && LangOpts.OpenCL)
+return getTargetInfo().getDataLayout().getAllocaAddrSpace();
Anastasia wrote:
>
Anastasia added a comment.
In https://reviews.llvm.org/D31404#714906, @yaxunl wrote:
> In https://reviews.llvm.org/D31404#714244, @Anastasia wrote:
>
> > I can't see clearly why the alloca has to be extended to accommodate the
> > address space too? Couldn't the address space for alloca just
yaxunl added a comment.
In https://reviews.llvm.org/D31404#714244, @Anastasia wrote:
> I can't see clearly why the alloca has to be extended to accommodate the
> address space too? Couldn't the address space for alloca just be taken
> directly from the data layout?
>
> In fact is seems from
Anastasia added a comment.
I can't see clearly why the alloca has to be extended to accommodate the
address space too? Couldn't the address space for alloca just be taken
directly from the data layout?
In fact is seems from the LLVM review, an address space for alloca doesn't go
into the
yaxunl marked an inline comment as done.
yaxunl added inline comments.
Comment at: include/clang/AST/Type.h:336-342
+ /// Get the address space value used in diagnostics.
+ unsigned getAddressSpacePrintValue() const {
+unsigned AS = getAddressSpace();
+if (AS >=
yaxunl marked an inline comment as done.
yaxunl added inline comments.
Comment at: include/clang/AST/ASTContext.h:2319
return AddrSpaceMapMangling ||
- AS < LangAS::Offset ||
- AS >= LangAS::Offset + LangAS::Count;
+ AS >
t-tye added inline comments.
Comment at: include/clang/AST/ASTContext.h:2319
return AddrSpaceMapMangling ||
- AS < LangAS::Offset ||
- AS >= LangAS::Offset + LangAS::Count;
+ AS > LangAS::target_first;
}
Should this be >=
yaxunl updated this revision to Diff 93329.
yaxunl added a comment.
Revised by Tony's comments.
https://reviews.llvm.org/D31404
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Type.h
include/clang/Basic/AddressSpaces.h
include/clang/Basic/DiagnosticSemaKinds.td
t-tye added inline comments.
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+return
yaxunl added inline comments.
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+return
t-tye added inline comments.
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+return
yaxunl added inline comments.
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+return
t-tye added inline comments.
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+return
yaxunl created this revision.
Herald added subscribers: nhaehnle, wdng.
There is an incoming change in LLVM allowing alloca to return a private pointer
which does not pointing to address space 0:
https://reviews.llvm.org/D31042#03b9d490
After this change is committed, alloca will return a
35 matches
Mail list logo
