[Builtins] Teach Clang about memccpy

[xbolva00]

Summary

Hopefully, -fno-builtin-memccpy will work now. Required for https://reviews.llvm.org/D67986.

[xbolva00]

include/clang/Basic/Builtins.def

903

Well, technically, C 20.

[aaron.ballman]

Test cases?

include/clang/Basic/Builtins.def

483

GCC doesn't seem to have __builtin_memccpy? https://godbolt.org/z/jbthQ3

[xbolva00]

It is not very obvious for me what kind of test should be done here. I would be grateful if you could show me an example in tree.

Exactly for -fno-builtin-memccpy case?

include/clang/Basic/Builtins.def

483

Ok, I will drop it.

[aaron.ballman]

In D68377#1693490, @xbolva00 wrote:

It is not very obvious for me what kind of test should be done here. I would be grateful if you could show me an example in tree.

I was thinking of something in CodeGen, like CodeGen/builtin-memfns.c

Exactly for -fno-builtin-memccpy case?

That would be another good test case.

include/clang/Basic/Builtins.def

483

If you drop it, won't that lose the builtin? I was mostly thinking it's in the wrong part of the list of builtins.

[xbolva00]

include/clang/Basic/Builtins.def

483

Rebuilding LLVM + Clang in progress so I just checked it in godbolt with "strtol" - defined only as LIBBUILTIN, no __builtin version.

nobuiltin attribute is correctly handled, so I think it will work.
https://godbolt.org/z/Olfv-w

[aaron.ballman]

include/clang/Basic/Builtins.def

483

Ah, I see the issue better now. The description for this review was very terse and it wasn't immediately clear what problem you were trying to solve.

Yeah, I think this declaration can go away.

984

Isn't memccpy supported by Visual Studio? What should happen there?

[xbolva00]

include/clang/Basic/Builtins.def

984

”This POSIX function is deprecated. Use the ISO C++ conformant _memccpy instead.“

[aaron.ballman]

include/clang/Basic/Builtins.def

984

Thanks for verifying it's still supported (deprecated != unsupported). We do support other Microsoft builtins, so should this one be added?

[xbolva00]

include/clang/Basic/Builtins.def

984

Ok, I will add it.

One last question, should I somehow represent that memccpy is in C 20 (if so, how) ?

[aaron.ballman]

include/clang/Basic/Builtins.def

984

You should probably start a new section for C2x library functions and add memccpy, strdup, and strndup to it.

Also, just to double-check: we don't encode restrict into builtin signatures, do we? (Should we?)

[xbolva00]

include/clang/Basic/Builtins.def

483

Yeah, I am sorry, more context why this is needed:

https://reviews.llvm.org/D67986

[xbolva00]

include/clang/Basic/Builtins.def

984

You should probably start a new section for C2x library functions and add memccpy, strdup, and strndup to it.

Okay, thanks - probably not needed for now (should be separate patch to handle new functions in C 2X anyway; maybe more functions to be added?)

Also, just to double-check: we don't encode restrict into builtin signatures, do we? (Should we?)

Maybe catch very simple overlap issues? I dont think this is worth it (we have sanitizers). LLVM annotates libc functions with noalias anyway.

[aaron.ballman]

include/clang/Basic/Builtins.def

483

Thank you for the extra context!

984

Okay, thanks - probably not needed for now (should be separate patch to handle new functions in C 2X anyway; maybe more functions to be added?)

Yeah, can totally be done in a separate patch.

Maybe catch very simple overlap issues? I dont think this is worth it (we have sanitizers). LLVM annotates libc functions with noalias anyway.

Sanitizers don't run everywhere, but more importantly, I was wondering about the possible optimization opportunities by noting which arguments cannot overlap with one another (perhaps noalias covers that; I'm less familiar with the llvm side of things).

However, I looked at the file and see we don't have any encodings for it, so nothing to be done here.

[xbolva00]

include/clang/Basic/Builtins.def

984

I was wondering about the possible optimization opportunities by noting which arguments cannot overlap with one another

This is implemented in LLVM.

For example: https://godbolt.org/z/E8x2Ev (see line 10).

Sanitizers don't run everywhere

GCC emits a lot of warnings from middle-end. While I dont like some implementations which are part of regular pass, but extra warning pass just to check specific thing is nice idea I think (that pass is enabled only with -Wxxxx flag).

[xbolva00]

Added some tests

[xbolva00]

Added it for msvc too.

[xbolva00]

Hm,
Maybe we have a bug in LLVM's TLI? TLI says Win32 has no memccpy.
https://llvm.org/doxygen/TargetLibraryInfo_8cpp_source.html line 333

cc @RKSimon since he knows windows-related things more.

[xbolva00]

Current solution does not work
/home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: error: redefinition of ‘BImemccpy’
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,

^

/home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: note: in definition of macro ‘BUILTIN’
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,

@aaron.ballman, maybe you can help me how to represent that memccpy exists for GNU and MS?

[xbolva00]

This comment was removed by xbolva00.

[xbolva00]

(still interested in this patch)

[xbolva00]

Ping @aaron.ballman

[aaron.ballman]

In D68377#1698399, @xbolva00 wrote:

Current solution does not work
/home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: error: redefinition of ‘BImemccpy’
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,

^

/home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: note: in definition of macro ‘BUILTIN’
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,

@aaron.ballman, maybe you can help me how to represent that memccpy exists for GNU and MS?

We may not have the machinery needed for doing this yet, in which case, I think it's fine to ignore the MS builtin.

[xbolva00]

Addressed review note.

[xbolva00]

Added new testcase

[aaron.ballman]

LGTM aside from a minor testing nit.

clang/test/CodeGen/memccpy-libcall.c

13 ↗

(On Diff #226571)

Please add the newline to the end of the file.

[xbolva00]

Newline.

include/clang/Basic/Builtins.def

 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")
 BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:")
 BUILTIN(__builtin_memchr, "v*vC*iz", "nF")
 BUILTIN(__builtin_memcmp, "ivC*vC*z", "nF")
 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF")
+BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF")
 BUILTIN(__builtin_memmove, "v*v*vC*z", "nF")
 BUILTIN(__builtin_mempcpy, "v*v*vC*z", "nF")
 BUILTIN(__builtin_memset, "v*v*iz", "nF")
 BUILTIN(__builtin_printf, "icC*.", "Fp:0:")
 BUILTIN(__builtin_stpcpy, "c*c*cC*", "nF")
 BUILTIN(__builtin_stpncpy, "c*c*cC*z", "nF")
 BUILTIN(__builtin_strcasecmp, "icC*cC*", "nF")
 BUILTIN(__builtin_strcat, "c*c*cC*", "nF")

 LIBBUILTIN(strtof, "fcC*c**",     "f",     "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(strtold, "LdcC*c**",   "f",     "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(strtol, "LicC*c**i",   "f",     "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(strtoll, "LLicC*c**i", "f",     "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(strtoul, "ULicC*c**i", "f",     "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(strtoull, "ULLicC*c**i", "f",   "stdlib.h", ALL_LANGUAGES)
 // C99 string.h
 LIBBUILTIN(memcpy, "v*v*vC*z",    "f",     "string.h", ALL_LANGUAGES)
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(memcmp, "ivC*vC*z",    "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(memmove, "v*v*vC*z",   "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(strcpy, "c*c*cC*",     "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(strncpy, "c*c*cC*z",   "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(strcmp, "icC*cC*",     "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(strncmp, "icC*cC*z",   "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(strcat, "c*c*cC*",     "f",     "string.h", ALL_LANGUAGES)
 LIBBUILTIN(strncat, "c*c*cC*z",   "f",     "string.h", ALL_LANGUAGES)

 LIBBUILTIN(longjmp, "vJi",        "fr",    "setjmp.h", ALL_LANGUAGES)
 
 // Non-C library functions, active in GNU mode only.
 // Functions with (returns_twice) attribute (marked as "j") are still active in
 // all languages, because losing this attribute would result in miscompilation
 // when these functions are used in non-GNU mode. PR16138.
 LIBBUILTIN(alloca, "v*z",         "f",     "stdlib.h", ALL_GNU_LANGUAGES)
 // POSIX string.h
 LIBBUILTIN(stpcpy, "c*c*cC*",     "f",     "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpncpy, "c*c*cC*z",   "f",     "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strdup, "c*cC*",       "f",     "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strndup, "c*cC*z",     "f",     "string.h", ALL_GNU_LANGUAGES)
 // POSIX strings.h
 LIBBUILTIN(index, "c*cC*i",       "f",     "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(rindex, "c*cC*i",      "f",     "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(bzero, "vv*z",         "f",     "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(bcmp, "ivC*vC*z",      "f",     "strings.h", ALL_GNU_LANGUAGES)