[Clang] Add attributes alloc_size and alloc_align to mm_malloc

[xbolva00]

Summary

LLVM optimizes source codes with mm_malloc better, especially due to alignment info.

alloc align https://clang.llvm.org/docs/AttributeReference.html#alloc-align
alloc size https://clang.llvm.org/docs/AttributeReference.html#alloc-size

[fhahn]

Not really familiar with testing for clang headers. Is it possible to test the new attributes have the desired effect?

[xbolva00]

Yes, test for alignment should be possible, something like

_Bool alig_test(void) {
// CHECK: ret i1 true
     yvoid *p = _mm_malloc(2014, 16);
    _Bool ret = ((__SIZE_TYPE__)p % 16) == 0;
    _mm_free(p);
    return ret;
}

For alloc size not sure how..

[xbolva00]

Added testcase

[xbolva00]

Ping @reames

[reames]

In D117091#3265986, @xbolva00 wrote:

Ping @reames

Not qualified to review clang/c++ library semantic changes.

Also, did we implement allocalign in LLVM? Last I knew it was a clang only attribute. If we did, it's missing from LangRef.

[xbolva00]

In D117091#3267980, @reames wrote:

In D117091#3265986, @xbolva00 wrote:

Ping @reames

Not qualified to review clang/c++ library semantic changes.

Also, did we implement allocalign in LLVM? Last I knew it was a clang only attribute. If we did, it's missing from LangRef.

I checked it and it is implemented as call site (alignment) assumption

[aaron.ballman]

There's a testing issue on Windows:

******************** TEST 'Clang :: Headers/mm_malloc.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe -emit-llvm -std=c11 -x c C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c -O1 --target=x86_64-linux-gnu -S -o - | c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c
--
Exit Code: 2

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe" "-emit-llvm" "-std=c11" "-x" "c" "C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c" "-O1" "--target=x86_64-linux-gnu" "-S" "-o" "-"
# command stderr:
In file included from C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c:2:

c:\ws\w2\llvm-project\premerge-checks\build\lib\clang\14.0.0\include\mm_malloc.h:13:10: fatal error: 'stdlib.h' file not found

#include <stdlib.h>

         ^~~~~~~~~~

1 error generated.


error: command failed with exit status: 1
$ "c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe" "C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c"
# command stderr:
FileCheck error: '<stdin>' is empty.

FileCheck command line:  c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c


error: command failed with exit status: 2

This one is neat because stdlib.h is not a header provided by the compiler but is instead provided by whatever CRT happens to be used. Normally, we require those headers to be mocked, but the goal here is to test the header the compiler does provide.

[jdoerfert]

Don't test with O1, add the dummy include folder to the include path (clang/test/Headers/Inputs/include), stdlib.h is already there, malloc.h is not and needs to be created.

See clang/test/Headers/nvptx_device_cmath_functions.c for an example.

[xbolva00]

Address review comments

[xbolva00]

Added newline

[xbolva00]

In D117091#3272594, @jdoerfert wrote:

Don't test with O1, add the dummy include folder to the include path (clang/test/Headers/Inputs/include), stdlib.h is already there, malloc.h is not and needs to be created.

See clang/test/Headers/nvptx_device_cmath_functions.c for an example.

Need -O1, with -O0 no assumption is emitted.

[xbolva00]

In D117091#3272589, @aaron.ballman wrote:

There's a testing issue on Windows:

******************** TEST 'Clang :: Headers/mm_malloc.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe -emit-llvm -std=c11 -x c C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c -O1 --target=x86_64-linux-gnu -S -o - | c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c
--
Exit Code: 2

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe" "-emit-llvm" "-std=c11" "-x" "c" "C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c" "-O1" "--target=x86_64-linux-gnu" "-S" "-o" "-"
# command stderr:
In file included from C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c:2:

c:\ws\w2\llvm-project\premerge-checks\build\lib\clang\14.0.0\include\mm_malloc.h:13:10: fatal error: 'stdlib.h' file not found

#include <stdlib.h>

         ^~~~~~~~~~

1 error generated.


error: command failed with exit status: 1
$ "c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe" "C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c"
# command stderr:
FileCheck error: '<stdin>' is empty.

FileCheck command line:  c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\mm_malloc.c


error: command failed with exit status: 2

This one is neat because stdlib.h is not a header provided by the compiler but is instead provided by whatever CRT happens to be used. Normally, we require those headers to be mocked, but the goal here is to test the header the compiler does provide.

Thanks for checking, I have no suitable Windows device to run patch on it.

[xbolva00]

ping

[aaron.ballman]

The header bits LGTM aside from a nit I found with potential identifier conflicts. I don't feel super qualified to review the behavioral aspects of the change, but if others are happy with that, I'm happy with the header changes.

clang/lib/Headers/mm_malloc.h

31–34

Protect against user macros.

[xbolva00]

protect against user macros

[xbolva00]

Thanks!

@jdoerfert ?

[xbolva00]

ping

clang/lib/Headers/mm_malloc.h

 // Sema::CheckEquivalentExceptionSpec, Clang accepts the following as a valid
 // redeclaration of glibc's declaration.
 extern "C" int posix_memalign(void **__memptr, size_t __alignment, size_t __size);
 #endif
 #endif
 
 #if !(defined(_WIN32) && defined(_mm_malloc))
 static __inline__ void *__attribute__((__always_inline__, __nodebug__,
-                                       __malloc__))
-_mm_malloc(size_t __size, size_t __align)
-{
+                                       __malloc__, __alloc_size__(1),
+                                       __alloc_align__(2)))
+_mm_malloc(size_t __size, size_t __align) {
   if (__align == 1) {
     return malloc(__size);
   }
 
   if (!(__align & (__align - 1)) && __align < sizeof(void *))
     __align = sizeof(void *);
 
   void *__mallocedMemory;
 #if defined(__MINGW32__)

clang/test/Headers/Inputs/include/malloc.h

+#if defined(__MINGW32__)
+void *__mingw_aligned_malloc(size_t, size_t);
+void __mingw_aligned_free(void *);
+#elif defined(_WIN32)
+void *_aligned_malloc(size_t, size_t);
+void _aligned_free(void *);
+#endif

clang/test/Headers/mm_malloc.c

+
+// RUN: %clang_cc1 -internal-isystem %S/Inputs/include %s -emit-llvm -O1 -triple x86_64-linux-gnu -o - | FileCheck %s
+#include <mm_malloc.h>
+
+_Bool align_test(void) {
+// CHECK-LABEL: @align_test(
+// CHECK:    ret i1 true
+     void *p = _mm_malloc(1024, 16);
+    _Bool ret = ((__UINTPTR_TYPE__)p % 16) == 0;
+    _mm_free(p);
+    return ret;
+}