On Mon, Aug 17, 2020 at 10:14 AM Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote: > > On Sun, Aug 16, 2020 at 8:02 AM Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > > > On Sun, Aug 16, 2020 at 07:22:35AM +0200, Sedat Dilek wrote: > > > On Sun, Aug 16, 2020 at 2:19 AM 'Fangrui Song' via Clang Built Linux > > > <clang-built-linux@xxxxxxxxxxxxxxxx> wrote: > > > > > > > > Adding a definition without a declaration for stpcpy looks good. > > > > Clang LTO will work. > > > > > > > > (If the kernel does not want to provide these routines, > > > > is http://git.kernel.org/linus/6edfba1b33c701108717f4e036320fc39abe1912 > > > > probably wrong? (why remove -ffreestanding from the main Makefile) ) > > > > > > > > > > We had some many issues in arch/x86 where *FLAGS were removed or used > > > differently and had to re-add them :-(. > > > > > > So if -ffreestanding is used in arch/x86 and was! used in top-level > > > Makefile - it makes sense to re-add it back? > > > ( I cannot speak for archs other than x86. ) > > > > > > - Sedat - > > > > -ffreestanding disables _all_ builtins and libcall optimizations, which > > is probably not desirable. If we added it back, we'd need to also go I agree. > > back to #define various string functions to the __builtin versions. > > > > Though I don't understand the original issue, with -ffreestanding, > > sprintf shouldn't have been turned into strcpy in the first place. Huh? The original issue for this thread is because `-ffreestanding` *isn't* being used for most targets (oh boy, actually mixed usage by ARCH. Looks like MIPS, m68k, superH, xtensa, and 32b x86 use it?); and I'm not suggesting it be used. > > 32-bit still has -ffreestanding -- I wonder if that's actually necessary > > any more? Fair question. Someone will have to go chase git history, since 0a6ef376d4ba covers it up. If anyone has any tricks to do so quickly; I'd love to know. I generally checkout the commit prior, then use vim fugitive to get git blame. > > Why does -fno-builtin-stpcpy not work with clang LTO? Isn't that a > > compiler bug? Yes; Sami found a recent patch that looks to me like it may have recently solved that bug. https://reviews.llvm.org/D71193 which landed Dec 12 2019. The bug report was based on https://github.com/ClangBuiltLinux/linux/issues/416#issuecomment-472231304 (Issue reported March 8 2019). And I do recall being able to reproduce the bug when I sent commit 5f074f3e192f ("lib/string.c: implement a basic bcmp") Now that that is fixed as reported by Sami below, I don't mind sending a revert for 5f074f3e192f that adds -fno-builtin-bcmp, because the current implementation of bcmp isn't useful. That said, this libcall optimization/transformation (sprintf->stpcpy) does look useful to me. Kees, do you have thoughts on me providing the implementation without exposing it in a header vs using -fno-builtin-stpcpy? (I would need to add the missing EXPORT_SYMBOL, as pointed out by 0day bot and on the github thread). I don't care either way; I'd just like your input before sending a V+1. Maybe better to just not implement it and never implement it? > > I just confirmed that adding -fno-builtin-stpcpy to KBUILD_CFLAGS does > work with LTO as well. > > Sami -- Thanks, ~Nick Desaulniers
