[PATCH] D29599: Clang Changes for alloc_align

[John McCall via Phabricator via cfe-commits]

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 indicated parameter. The
+alignment parameter is one-based. In the case of member functions, the
----------------
"that the return value of the function ... is at least as aligned as the
value of the indicated parameter.  The parameter is given by its index
in the list of formal parameters; the first parameter has index 1 unless
the function is a C++ non-static member function, in which case the
first parameter has index 2 to account for the implicit ``this`` parameter."

What's the behavior of this attribute if the given attribute is not a power of 2?
Does it silently have no effect, or is it U.B.?


================
Comment at: include/clang/Basic/AttrDocs.td:265
+  // The returned pointer has the alignment specified by the second visible
+  // parameter, however it must be adjusted for the implicit 'this' parameter.
+  void *Foo::b(void *v, size_t align) __attribute__((alloc_align(3)));
----------------
"The returned pointer has the alignment specified by the second formal
parameter, 'align'.  However, for the purposes of the alloc_align attribute,
the parameter index is 3 because of the implicit 'this' parameter."


================
Comment at: include/clang/Basic/AttrDocs.td:270
+condition that the code already ensures is true. It does not cause the compiler
+to enforce the provided alignment assumption.
+  }];
----------------
"Note that this attribute merely informs the compiler that a function always
returns a sufficiently aligned pointer.  It does not cause the compiler to emit
code to enforce that alignment.  The behavior is undefined if the returned
pointer is not sufficiently aligned."


================
Comment at: lib/CodeGen/CGCall.cpp:4352
+    } else if (const auto *AA = TargetDecl->getAttr<AllocAlignAttr>()) {
+      llvm::Value *ParamVal = IRCallArgs[AA->getParamIndex() - 1];
+      EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);
----------------
IRCallArgs is not one-to-one with the list of formal parameters.  You need to be taking the value out of CallArgList.  Three test cases come to mind:

1. There's an earlier struct argument which expands to no IR arguments.
2. There's an earlier struct argument which expands to multiple IR arguments.
3. The alignment argument itself expands to multiple IR arguments.  For example, __int128_t on x86-64.


https://reviews.llvm.org/D29599