[PATCH] Support the assume_aligned function attribute
Richard Smith
richard at metafoo.co.uk
Sun Jul 20 16:25:34 PDT 2014
Somewhat unrelated to this patch in particular, but we should add a UBSan check to catch failed assumptions.
================
Comment at: include/clang/Basic/Attr.td:867
@@ +866,3 @@
+ "ExpectedFunctionOrMethod">;
+ let Args = [IntArgument<"Alignment">, DefaultIntArgument<"Offset", 0>];
+ let Documentation = [Undocumented];
----------------
These should probably both be `ExprArgument`s, in order to support dependent alignment/offset expressions in templates.
================
Comment at: include/clang/Basic/Attr.td:868
@@ +867,3 @@
+ let Args = [IntArgument<"Alignment">, DefaultIntArgument<"Offset", 0>];
+ let Documentation = [Undocumented];
+}
----------------
Please add documentation.
================
Comment at: lib/CodeGen/CGExpr.cpp:3352
@@ +3351,3 @@
+
+ if (Ret.isScalar() && TargetDecl)
+ if (const AssumeAlignedAttr *AA =
----------------
Please add parens around this `if`.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1219-1227
@@ +1218,11 @@
+
+ if (Attr.getNumArgs() > 2) {
+ S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
+ << Attr.getName() << 2;
+ return;
+ } else if (Attr.getNumArgs() < 1) {
+ S.Diag(Attr.getLoc(), diag::err_attribute_too_few_arguments)
+ << Attr.getName() << 1;
+ return;
+ }
+
----------------
I think there's a helper function for this sequence of checks/diagnostics. If not, there should be.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1232-1235
@@ +1231,6 @@
+ return;
+ if (Attr.getNumArgs() > 1 && !checkUInt32Argument(S, Attr,
+ Attr.getArgAsExpr(1),
+ Offset, 2))
+ return;
+
----------------
Do you really need to check that the `Offset` fits into an `uint32_t`? Truncating the offset to the width of `Alignment` seems correct in all cases.
http://reviews.llvm.org/D4601
More information about the cfe-commits
mailing list