On Mon, Aug 17, 2020 at 10:14 AM Sami Tolvanen samitolvanen@google.com wrote:
On Sun, Aug 16, 2020 at 8:02 AM Arvind Sankar nivedita@alum.mit.edu 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@googlegroups.com 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