[PATCH] Support the assume_aligned function attribute

[Aaron Ballman]

On Sun, Jul 20, 2014 at 7:25 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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.

They aren't required -- the common handler takes care of that stuff.

>
> ================
> 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.

This doesn't check that it fits into a uint32_t. It checks that it's a
valid integer constant expression and errs if it's not. It then gets
you the value as a uint32_t.

~Aaron