This revision was automatically updated to reflect the committed changes.
Closed by commit rL299117: Clang changes for alloc_align attribute (authored
by erichkeane).
Changed prior to commit:
https://reviews.llvm.org/D29599?vs=93537=93549#toc
Repository:
rL LLVM
erichkeane added a comment.
Thanks guys. I THINK I properly removed the svn properties properly, though, I
actually didn't know they existed until just now!
Repository:
rL LLVM
https://reviews.llvm.org/D29599
___
cfe-commits mailing list
aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM!
Can you drop the svn props on the new files when you commit? I don't think we
usually set them, and I've seen commits specifically removing the eol-style
props before.
https://reviews.llvm.org/D29599
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks. With that, LGTM.
https://reviews.llvm.org/D29599
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
erichkeane updated this revision to Diff 93537.
erichkeane added a comment.
Added tests as requested by John.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
lib/CodeGen/CGCall.cpp
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:4363
+} else if (AllocAlignParam && TargetDecl->hasAttr())
+ EmitAlignmentAssumption(Ret.getScalarVal(), AllocAlignParam);
}
rjmccall wrote:
> Your old code was fine, you just needed
erichkeane updated this revision to Diff 93491.
erichkeane marked 3 inline comments as done.
erichkeane added a comment.
Thanks for the feedback John!
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
rjmccall added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) is at least as aligned as the value indicated parameter. The
+parameter is given by its index in the
erichkeane updated this revision to Diff 93421.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.
Fixes based on John's comments. A little bit of extra work was required to get
the correct Value from the attribute, but impact was minimal.
https://reviews.llvm.org/D29599
erichkeane marked 2 inline comments as done.
erichkeane added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter. The
rjmccall added a comment.
I see that GCC is up to its same parameter-indexing shenanigans again.
Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the
erichkeane updated this revision to Diff 93409.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
lib/CodeGen/CGCall.cpp
lib/CodeGen/CodeGenFunction.h
lib/Sema/SemaDeclAttr.cpp
erichkeane marked 9 inline comments as done.
erichkeane added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:1608-1612
+ IndexVal += 1 + isInstanceMethod(FuncDecl);
+
+ if (!checkParamIsIntegerType(*this, FuncDecl, TmpAttr, ParamExpr, IndexVal,
+
aaron.ballman added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter. The
+alignment parameter is one-indexed. In
erichkeane updated this revision to Diff 93404.
erichkeane added a comment.
Made the changes as requested. checkFunctionOrMethodParameterIndex corrects
for 1->0 index and implicit this, which requires undoing, otherwise templates
create a big hassle. Additionally, please note the AttrDocs
erichkeane marked 2 inline comments as done.
erichkeane added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:252
+declaration to specify that the return value of the function (which must be a
+pointer type) has an alignment specified by the indicated parameter,
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:1230
+ let Spellings = [GCC<"alloc_align">];
+ let Subjects = SubjectList<[ Function]>;
+ let Args = [IntArgument<"ParamIndex">];
There's a spurious space between [ and Function.
If
erichkeane updated this revision to Diff 93158.
erichkeane marked 2 inline comments as done.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
lib/CodeGen/CGCall.cpp
lib/CodeGen/CodeGenFunction.h
erichkeane marked 15 inline comments as done.
erichkeane added a comment.
Comments on 2 cases, otherwise a Patch incoming that fixes the rest of Aaron's
comments.
Comment at: include/clang/Basic/Attr.td:1224
+def AllocAlign : InheritableAttr {
+ let Spellings = [
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:1224
+def AllocAlign : InheritableAttr {
+ let Spellings = [ GCC<"alloc_align"> ];
+ let Subjects = SubjectList<[Function]>;
Extra spaces in the declaration that do not match the
erichkeane added a comment.
FWIW, the LLVM changes for this have been committed.
https://reviews.llvm.org/D29599
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane updated this revision to Diff 92814.
erichkeane added a comment.
Update test based on the corresponding LLVM change.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
lib/CodeGen/CGCall.cpp
erichkeane updated this revision to Diff 92709.
erichkeane marked an inline comment as done.
erichkeane added a comment.
Thanks for the review! Tests updated to clarify the cast htat is happening.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
myatsina added inline comments.
Comment at: test/CodeGen/alloc-align-attr.c:19
+}
+// Condition where test2 param needs casting.
+__INT32_TYPE__ test2(__SIZE_TYPE__ a) {
Where exactly do we see this test2 param casting?
I think you have a missing check before
erichkeane added a comment.
Ping!
https://reviews.llvm.org/D29599
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane updated this revision to Diff 88602.
erichkeane added a comment.
I was able to get the templated versions working in response to the discussion
with Akira. Note the added test file which shows off all of the combos I could
think of.
It required a little bit of surgery inside the
erichkeane added a comment.
Ah, I see! Sorry for missing that. I don't see a reason why we cannot support
that, but I wasn't really considering it. In general, this attribute is a
compiler hint for some C standard library stuff in glibc.
I've been playing with it a few hours now, and it
ahatanak added a comment.
My question probably wasn't clear, but I wasn't sure how template functions in
general (not just member functions) should be handled.
I see a warning when the following function is compiled:
template
T __attribute__((alloc_align(1))) foo0(int a) {
typedef
erichkeane added a comment.
In https://reviews.llvm.org/D29599#674772, @ahatanak wrote:
> Can this attribute be used on c++ template methods? Is the following code
> valid?
>
> template
> struct S {
> T foo(int a) __attribute__((alloc_align(1)));
> };
>
Yes it can, however that
ahatanak added a comment.
Can this attribute be used on c++ template methods? Is the following code valid?
template
struct S {
T foo(int a) __attribute__((alloc_align(1)));
};
https://reviews.llvm.org/D29599
___
cfe-commits mailing list
erichkeane added a comment.
Requires this LLVM commit: https://reviews.llvm.org/D29598
https://reviews.llvm.org/D29599
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane created this revision.
GCC has the alloc_align attribute, which is similar to assume_aligned, except
the attribute's parameter is the index of the integer parameter that needs
aligning to.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
32 matches
Mail list logo
