[PATCH] Support the assume_aligned function attribute

[hfinkel at anl.gov]

================
Comment at: lib/CodeGen/CGExpr.cpp:3379-3382
@@ -3378,2 +3378,6 @@
 
-  return EmitCall(FnInfo, Callee, ReturnValue, Args, TargetDecl);
+  RValue Ret = EmitCall(FnInfo, Callee, ReturnValue, Args, TargetDecl);
+
+  if (Ret.isScalar() && TargetDecl) {
+    if (const auto *AA = TargetDecl->getAttr<AssumeAlignedAttr>()) {
+      llvm::Value *OffsetValue = nullptr;
----------------
rsmith wrote:
> Is this really the right place for this code, rather than in the called `EmitCall` function? Various places call the lower-level overload directly.
When I first looked, I thought that there were no other relevant callers, but maybe EmitCallAndReturnForThunk and EmitForwardingCallToLambda would need this handling too?

Because of the way that the lower-level EmitCall is structured, I'd need to wrap it (it has too many early returns). What would I call it?

================
Comment at: lib/Sema/SemaDeclAttr.cpp:1130
@@ -1129,2 +1129,3 @@
+bool Sema::isValidPointerAttrType(QualType T) {
   T = T.getNonReferenceType();
 
----------------
rsmith wrote:
> Is this really appropriate for your attribute? (It makes almost no sense for `__attribute__((nonnull))` either, but I couldn't find anyone at Apple to tell me what the relevant radar says, so I have no idea why we have this "feature".)
Nice :-) -- Honestly, I don't have a strong opinion regarding whether or not we strip references, but I like the idea of consistency between the two attributes. The set of returned pointers about which I can assert something about the alignment should be the same as that about which I can assert a nonnull property.

That having been said, it looks like the CodeGen for returns_nonnull that adds the nonnull IR attribute (and the associated UBSan code) does not handle references-to-pointers correctly. I'll fix that as a follow-up too.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:203-205
@@ +202,5 @@
+    const AssumeAlignedAttr *AssumeAligned = dyn_cast<AssumeAlignedAttr>(TmplAttr);
+    if (AssumeAligned && (AssumeAligned->getAlignment()->isValueDependent() ||
+                          (AssumeAligned->getOffset() &&
+                           AssumeAligned->getOffset()->isValueDependent()))) {
+      instantiateDependentAssumeAlignedAttr(*this, TemplateArgs, AssumeAligned, New);
----------------
rsmith wrote:
> You shouldn't have these 'is dependent' checks here; the other code above and below is wrong to include them. Testcase:
> 
>   template<typename T> __attribute__((assume_aligned(sizeof(int(T()))))) T *f();
>   void *p = f<void>();
> 
> This should result in a hard error, because `int(void())` is invalid, but I think you'll accept it, because `sizeof(int(T()))` is not value-dependent, so you never actually perform the substitution.
Interestingly, the current code does generate an error:
  invalid application of 'sizeof' to a function type

Removing the type-dependent checks does not seem to change any of the current behavior.

http://reviews.llvm.org/D4601