[PATCH] D100879: [Clang] Propagate guaranteed alignment for malloc and others

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Test looks good, thanks.



================
Comment at: clang/lib/CodeGen/CGCall.cpp:2060
+        case Builtin::BIstrndup:
+          RetAttrs.addAlignmentAttr(Context.getTargetInfo().getNewAlign() /
+                                    Context.getTargetInfo().getCharWidth());
----------------
xbolva00 wrote:
> lebedev.ri wrote:
> > malloc != new
> But the comment for the implementation of this value points on eg. gcc's docs:
> 
> https://clang.llvm.org/doxygen/Basic_2TargetInfo_8cpp_source.html line 67
> 
>    // From the glibc documentation, on GNU systems, malloc guarantees 16-byte
>    // alignment on 64-bit systems and 8-byte alignment on 32-bit systems. See
>    // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html.
>    // This alignment guarantee also applies to Windows and Android. On Darwin,
>    // the alignment is 16 bytes on both 64-bit and 32-bit systems.
> 
> So this value is correct to be used for malloc and friends IMHO.
There's no immediate reason to have two different methods, but we should at least update the comment on `getNewAlign` to document that we're assuming it describes both `operator new` and `malloc`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100879/new/

https://reviews.llvm.org/D100879