Map -O to -O1 instead of -O2

[MaskRay]

Summary

rL82131 changed -O from -O1 to -O2, because -O1 was not different from
-O2 at that time.

GCC treats -O as -O1 and there is now work to make -O1 meaningful.
We can change -O back to -O1 again.

[hans]

Actually it was http://llvm.org/r82131 that mapped -O to -O2 (I just refactored it). Originally it seems it was mapped to -O1.

Because this seems to have done quite intentionally, I'm a little vary of changing it back. I'm not sure who would be a good person to have insights here though.

+echristo maybe?

[MaskRay]

Update description

[echristo]

I'm totally down, but you knew that already :)

Duncan: Do you have any concerns? I doubt it, but just checking.

[dexonsmith]

In D79916#2039789, @echristo wrote:

I'm totally down, but you knew that already :)

Duncan: Do you have any concerns? I doubt it, but just checking.

Xcode doesn't use -O. There could be some internal users, but I doubt it, and we can probably migrate them if this causes a problem. @arphaman, WDYT?

@Gerolf, I don't imagine you have any concerns, but thought I should double-check.

[dexonsmith]

IOW, this LGTM if Alex and Gerolf are happy.

[arphaman]

In D79916#2039842, @dexonsmith wrote:

IOW, this LGTM if Alex and Gerolf are happy.

LGTM

[dexonsmith]

In D79916#2039842, @dexonsmith wrote:

IOW, this LGTM if Alex and Gerolf are happy.

Gerolf told me he has no concerns.

[MaskRay]

In D79916#2042782, @dexonsmith wrote:

In D79916#2039842, @dexonsmith wrote:

IOW, this LGTM if Alex and Gerolf are happy.

Gerolf told me he has no concerns.

Thanks! (I was just about to ask whether you have a quick channel to Gerolf :)

I'll re-test and commit.

[jrtc27]

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

[Bdragon28]

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to request -O2 at all times when building powerpc/powerpcspe just to get a functioning kernel.

[jrtc27]

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

[Bdragon28]

In D79916#2279816, @jrtc27 wrote:

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

As an example:

static __inline void
mtsrin(vm_offset_t va, register_t value)
{

        __asm __volatile ("mtsrin %0,%1; isync" :: "r"(value), "r"(va));
}

This code is used in the mmu when bootstrapping the cpu like so:

for (i = 0; i < 16; i++)
        mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();

sdr = (u_int)moea_pteg_table | (moea_pteg_mask >> 10);
__asm __volatile("mtsdr1 %0" :: "r"(sdr));
isync();

tlbia();

During the loop there, we are in the middle of programming the MMU segment registers in real mode, and is supposed to be doing all work out of registers. (and powerpc_sync() and isync() should be expanded to their single assembly instruction, not a function call. The whole point of calling those is that we are in an inconsistent hardware state and need to sync up before continuing execution)

If there isn't a way to force inlining, we will have to change to using preprocessor macros in cpufunc.h.

[jrtc27]

In D79916#2279863, @Bdragon28 wrote:

In D79916#2279816, @jrtc27 wrote:

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

As an example:

static __inline void
mtsrin(vm_offset_t va, register_t value)
{

        __asm __volatile ("mtsrin %0,%1; isync" :: "r"(value), "r"(va));
}

This code is used in the mmu when bootstrapping the cpu like so:

for (i = 0; i < 16; i++)
        mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();

sdr = (u_int)moea_pteg_table | (moea_pteg_mask >> 10);
__asm __volatile("mtsdr1 %0" :: "r"(sdr));
isync();

tlbia();

During the loop there, we are in the middle of programming the MMU segment registers in real mode, and is supposed to be doing all work out of registers. (and powerpc_sync() and isync() should be expanded to their single assembly instruction, not a function call. The whole point of calling those is that we are in an inconsistent hardware state and need to sync up before continuing execution)

If there isn't a way to force inlining, we will have to change to using preprocessor macros in cpufunc.h.

There is, it's called __attribute__((always_inline)) and supported by both GCC and Clang. But at -O0 you'll still have register allocation to deal with, so really that code is just fundamentally broken and should not be written in C. There is no way for you to guarantee stack spills are not used, it's way out of scope for C.

[Bdragon28]

In D79916#2279863, @Bdragon28 wrote:

In D79916#2279816, @jrtc27 wrote:

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

As an example:

static __inline void
mtsrin(vm_offset_t va, register_t value)
{

        __asm __volatile ("mtsrin %0,%1; isync" :: "r"(value), "r"(va));
}

This code is used in the mmu when bootstrapping the cpu like so:

for (i = 0; i < 16; i++)
        mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();

sdr = (u_int)moea_pteg_table | (moea_pteg_mask >> 10);
__asm __volatile("mtsdr1 %0" :: "r"(sdr));
isync();

tlbia();

During the loop there, we are in the middle of programming the MMU segment registers in real mode, and is supposed to be doing all work out of registers. (and powerpc_sync() and isync() should be expanded to their single assembly instruction, not a function call. The whole point of calling those is that we are in an inconsistent hardware state and need to sync up before continuing execution)

If there isn't a way to force inlining, we will have to change to using preprocessor macros in cpufunc.h.

Actually, this is probably a bad example. Since we're in real mode it doesn't really matter. But I can see other places where powerpc_sync() / isync() are dangerous to expand to a function call.

[Bdragon28]

In D79916#2279866, @jrtc27 wrote:

In D79916#2279863, @Bdragon28 wrote:

In D79916#2279816, @jrtc27 wrote:

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

As an example:

static __inline void
mtsrin(vm_offset_t va, register_t value)
{

        __asm __volatile ("mtsrin %0,%1; isync" :: "r"(value), "r"(va));
}

This code is used in the mmu when bootstrapping the cpu like so:

for (i = 0; i < 16; i++)
        mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();

sdr = (u_int)moea_pteg_table | (moea_pteg_mask >> 10);
__asm __volatile("mtsdr1 %0" :: "r"(sdr));
isync();

tlbia();

During the loop there, we are in the middle of programming the MMU segment registers in real mode, and is supposed to be doing all work out of registers. (and powerpc_sync() and isync() should be expanded to their single assembly instruction, not a function call. The whole point of calling those is that we are in an inconsistent hardware state and need to sync up before continuing execution)

If there isn't a way to force inlining, we will have to change to using preprocessor macros in cpufunc.h.

There is, it's called __attribute__((always_inline)) and supported by both GCC and Clang. But at -O0 you'll still have register allocation to deal with, so really that code is just fundamentally broken and should not be written in C. There is no way for you to guarantee stack spills are not used, it's way out of scope for C.

Is there a way to have always_inline and unused at the same time? I tried using always_inline and it caused warnings in things that used *parts* of cpufunc.h.

[jrtc27]

(and FreeBSD has an __always_inline in sys/sys/cdef.s like __inline)

[jrtc27]

In D79916#2279871, @Bdragon28 wrote:

In D79916#2279866, @jrtc27 wrote:

In D79916#2279863, @Bdragon28 wrote:

In D79916#2279816, @jrtc27 wrote:

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

As an example:

static __inline void
mtsrin(vm_offset_t va, register_t value)
{

        __asm __volatile ("mtsrin %0,%1; isync" :: "r"(value), "r"(va));
}

This code is used in the mmu when bootstrapping the cpu like so:

for (i = 0; i < 16; i++)
        mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();

sdr = (u_int)moea_pteg_table | (moea_pteg_mask >> 10);
__asm __volatile("mtsdr1 %0" :: "r"(sdr));
isync();

tlbia();

During the loop there, we are in the middle of programming the MMU segment registers in real mode, and is supposed to be doing all work out of registers. (and powerpc_sync() and isync() should be expanded to their single assembly instruction, not a function call. The whole point of calling those is that we are in an inconsistent hardware state and need to sync up before continuing execution)

If there isn't a way to force inlining, we will have to change to using preprocessor macros in cpufunc.h.

There is, it's called __attribute__((always_inline)) and supported by both GCC and Clang. But at -O0 you'll still have register allocation to deal with, so really that code is just fundamentally broken and should not be written in C. There is no way for you to guarantee stack spills are not used, it's way out of scope for C.

Is there a way to have always_inline and unused at the same time? I tried using always_inline and it caused warnings in things that used *parts* of cpufunc.h.

Both __attribute__((always_inline)) __attribute__((unused)) and __attribute__((always_inline, unused)) work, but really you should use __always_inline __unused in FreeBSD (which will expand to the former).

[jrtc27]

In D79916#2279875, @jrtc27 wrote:

In D79916#2279871, @Bdragon28 wrote:

In D79916#2279866, @jrtc27 wrote:

In D79916#2279863, @Bdragon28 wrote:

In D79916#2279816, @jrtc27 wrote:

In D79916#2279812, @Bdragon28 wrote:

In D79916#2279045, @jrtc27 wrote:

This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kernel with -O whenever debug symbols are enabled[1] (which, of course, is almost always true), this results in all its static inline helper functions not being inlined at all, a pattern that is common in the kernel, used for things like get_curthread and the atomics implementations.

[1] This is a dubious decision made in r140400 in 2005 to provide "truer debugger stack traces" (well, before then there was ping-ponging between -O and -O2 based on concerns around correctness vs performance, but amd64 is an exception that has always used -O2 since r127180 it seems). Given that GCC will inline at -O, at least these days, the motivation seems to no longer exist, and compiling a kernel at anything other than -O2 (or maybe -O3) seems like a silly thing to do, but nevertheless it's what is currently done.

Cc: @dim @trasz

This is actually SUCH a bad idea that a kernel built with -O will *not work at all* on 32 bit powerpc platforms (presumably due to allocating stack frames in the middle of assembly fragments in the memory management that are supposed to be inlined at all times.) I had to hack kern.pre.mk to rquest -O2 at all times just to get a functioning kernel.

Well, -O0, -O1, -O2 and -O should all produce working kernels, and any cases where they don't are compiler bugs (or kernel bugs if they rely on UB) that should be fixed, not worked around by tweaking the compiler flags in a fragile way until you get the behaviour relied on. Correctness and performance are very different issues here.

As an example:

static __inline void
mtsrin(vm_offset_t va, register_t value)
{

        __asm __volatile ("mtsrin %0,%1; isync" :: "r"(value), "r"(va));
}

This code is used in the mmu when bootstrapping the cpu like so:

for (i = 0; i < 16; i++)
        mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();

sdr = (u_int)moea_pteg_table | (moea_pteg_mask >> 10);
__asm __volatile("mtsdr1 %0" :: "r"(sdr));
isync();

tlbia();

During the loop there, we are in the middle of programming the MMU segment registers in real mode, and is supposed to be doing all work out of registers. (and powerpc_sync() and isync() should be expanded to their single assembly instruction, not a function call. The whole point of calling those is that we are in an inconsistent hardware state and need to sync up before continuing execution)

If there isn't a way to force inlining, we will have to change to using preprocessor macros in cpufunc.h.

There is, it's called __attribute__((always_inline)) and supported by both GCC and Clang. But at -O0 you'll still have register allocation to deal with, so really that code is just fundamentally broken and should not be written in C. There is no way for you to guarantee stack spills are not used, it's way out of scope for C.

Is there a way to have always_inline and unused at the same time? I tried using always_inline and it caused warnings in things that used *parts* of cpufunc.h.

Both __attribute__((always_inline)) __attribute__((unused)) and __attribute__((always_inline, unused)) work, but really you should use __always_inline __unused in FreeBSD (which will expand to the former).

But also you really should not get warnings for unused static functions in included headers, only ones defined in the C source file itself. We'd have countless warnings in the kernel across all architectures otherwise.

[Bdragon28]

In D79916#2279884, @jrtc27 wrote:

But also you really should not get warnings for unused static functions in included headers, only ones defined in the C source file itself. We'd have countless warnings in the kernel across all architectures otherwise.

I agree. But that's what it is doing when using always_inline in combination with -Wunused-function.

There is currently no real usage of always_inline in system headers though, so maybe I'm just the first to complain about it?

[jrtc27]

In D79916#2279901, @Bdragon28 wrote:

In D79916#2279884, @jrtc27 wrote:

But also you really should not get warnings for unused static functions in included headers, only ones defined in the C source file itself. We'd have countless warnings in the kernel across all architectures otherwise.

I agree. But that's what it is doing when using always_inline in combination with -Wunused-function.

There is currently no real usage of always_inline in system headers though, so maybe I'm just the first to complain about it?

We use them in CheriBSD and have no such issues that I've ever noticed. When was the last time you checked (and what compiler)?

[Bdragon28]

In D79916#2279918, @jrtc27 wrote:

In D79916#2279901, @Bdragon28 wrote:

In D79916#2279884, @jrtc27 wrote:

But also you really should not get warnings for unused static functions in included headers, only ones defined in the C source file itself. We'd have countless warnings in the kernel across all architectures otherwise.

I agree. But that's what it is doing when using always_inline in combination with -Wunused-function.

There is currently no real usage of always_inline in system headers though, so maybe I'm just the first to complain about it?

We use them in CheriBSD and have no such issues that I've ever noticed. When was the last time you checked (and what compiler)?

Five minutes ago, FreeBSD clang version 11.0.0 ([e-mail]:llvm/llvm-project.git llvmorg-11.0.0-rc2-0-g414f32a9e86)

Using always_inline unused appears to work to silence the warnings, however, so that is probably workable.

[MaskRay]

Several previous comments are FreeBSD specific. To we clang developers, the concrete request is

Given that GCC will inline at -O, at least these days, ...

right? I think this makes sense, especially when inline is explicitly specified... This appears to be related to some -O1 work @echristo is working on.

// gcc -O1 and g++ -O1 inline `foo`. Note that in C99 mode, `extern int foo` is needed to ask the compiler to provide an external definition.
// clang -O1 and clang++ -O1 do not inline `foo`
inline int foo(int a) {
  return a + a;
}

int bar(int a, int b) {
  return foo(a + b);
}

[jrtc27]

In D79916#2279987, @MaskRay wrote:

Several previous comments are FreeBSD specific. To we clang developers, the concrete request is

Given that GCC will inline at -O, at least these days, ...

right? I think this makes sense, especially when inline is explicitly specified... This appears to be related to some -O1 work @echristo is working on.

// gcc -O1 and g++ -O1 inline `foo`. Note that in C99 mode, `extern int foo` is needed to ask the compiler to provide an external definition.
// clang -O1 and clang++ -O1 do not inline `foo`
inline int foo(int a) {
  return a + a;
}

int bar(int a, int b) {
  return foo(a + b);
}

Yes, inline should certainly be inlined, but also non-inline things should too. Perhaps not so aggressively, but there's no good reason not to, really, it can be a big win with a simple transformation. GCC seems to inline even those:

# echo 'static void foo(void) { __asm__ __volatile__ ("#asdf"); } void bar(void) { foo(); } void baz(void) { foo(); foo(); }' | gcc -x c - -o - -S -O1
	.file	""
	.text
	.globl	bar
	.type	bar, @function
bar:
.LFB1:
	.cfi_startproc
#APP
# 1 "<stdin>" 1
	#asdf
# 0 "" 2
#NO_APP
	ret
	.cfi_endproc
.LFE1:
	.size	bar, .-bar
	.globl	baz
	.type	baz, @function
baz:
.LFB2:
	.cfi_startproc
#APP
# 1 "<stdin>" 1
	#asdf
# 0 "" 2
# 1 "<stdin>" 1
	#asdf
# 0 "" 2
#NO_APP
	ret
	.cfi_endproc
.LFE2:
	.size	baz, .-baz
	.ident	"GCC: (Debian 10.2.0-8) 10.2.0"
	.section	.note.GNU-stack,"",@progbits

clang/include/clang/Driver/Options.td

 def Mach : Flag<["-"], "Mach">, Group<Link_Group>;
 def O0 : Flag<["-"], "O0">, Group<O_Group>, Flags<[CC1Option, HelpHidden]>;
 def O4 : Flag<["-"], "O4">, Group<O_Group>, Flags<[CC1Option, HelpHidden]>;
 def ObjCXX : Flag<["-"], "ObjC++">, Flags<[DriverOption]>,
   HelpText<"Treat source input files as Objective-C++ inputs">;
 def ObjC : Flag<["-"], "ObjC">, Flags<[DriverOption]>,
   HelpText<"Treat source input files as Objective-C inputs">;
 def O : Joined<["-"], "O">, Group<O_Group>, Flags<[CC1Option]>;
-def O_flag : Flag<["-"], "O">, Flags<[CC1Option]>, Alias<O>, AliasArgs<["2"]>;
+def O_flag : Flag<["-"], "O">, Flags<[CC1Option]>, Alias<O>, AliasArgs<["1"]>;
 def Ofast : Joined<["-"], "Ofast">, Group<O_Group>, Flags<[CC1Option]>;
 def P : Flag<["-"], "P">, Flags<[CC1Option]>, Group<Preprocessor_Group>,
   HelpText<"Disable linemarker output in -E mode">;
 def Qy : Flag<["-"], "Qy">, Flags<[CC1Option]>,
   HelpText<"Emit metadata containing compiler name and version">;
 def Qn : Flag<["-"], "Qn">, Flags<[CC1Option]>,
   HelpText<"Do not emit metadata containing compiler name and version">;
 def : Flag<["-"], "fident">, Group<f_Group>, Alias<Qy>, Flags<[CC1Option]>;

clang/lib/Frontend/CompilerInvocation.cpp

if (A->getOption().matches(options::OPT_O0))
       return llvm::CodeGenOpt::None;
 
     if (A->getOption().matches(options::OPT_Ofast))
       return llvm::CodeGenOpt::Aggressive;
 
     assert(A->getOption().matches(options::OPT_O));
 
     StringRef S(A->getValue());
-    if (S == "s" || S == "z" || S.empty())
+    if (S == "s" || S == "z")
       return llvm::CodeGenOpt::Default;
 
     if (S == "g")
       return llvm::CodeGenOpt::Less;
 
     return getLastArgIntValue(Args, OPT_O, DefaultOpt, Diags);
   }
 

clang/test/CodeGen/builtins-systemz-zvector-constrained.c

 // REQUIRES: systemz-registered-target
 // RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -ffp-exception-behavior=strict \
 // RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -ffp-exception-behavior=strict \
 // RUN: -Wall -Wno-unused -Werror -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 #include <vecintrin.h>
 
 volatile vector signed long long vsl;
 volatile vector unsigned long long vul;
 volatile vector bool long long vbl;

clang/test/CodeGen/builtins-systemz-zvector.c

 // REQUIRES: systemz-registered-target
 // RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -Wall -Wno-unused -Werror -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 #include <vecintrin.h>
 
 volatile vector signed char vsc;
 volatile vector signed short vss;
 volatile vector signed int vsi;
 volatile vector signed long long vsl;

clang/test/CodeGen/builtins-systemz-zvector2-constrained.c

 // REQUIRES: systemz-registered-target
 // RUN: %clang_cc1 -target-cpu z14 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -ffp-exception-behavior=strict \
 // RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -target-cpu z14 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -ffp-exception-behavior=strict \
 // RUN: -Wall -Wno-unused -Werror -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 #include <vecintrin.h>
 
 volatile vector signed long long vsl;
 volatile vector unsigned int vui;
 volatile vector unsigned long long vul;

clang/test/CodeGen/builtins-systemz-zvector2.c

 // REQUIRES: systemz-registered-target
 // RUN: %clang_cc1 -target-cpu z14 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -target-cpu z14 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -Wall -Wno-unused -Werror -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 #include <vecintrin.h>
 
 volatile vector signed char vsc;
 volatile vector signed short vss;
 volatile vector signed int vsi;
 volatile vector signed long long vsl;

clang/test/CodeGen/builtins-systemz-zvector3-constrained.c

 // REQUIRES: systemz-registered-target
 // RUN: %clang_cc1 -target-cpu z15 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -ffp-exception-behavior=strict \
 // RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -target-cpu z15 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -ffp-exception-behavior=strict \
 // RUN: -Wall -Wno-unused -Werror -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 #include <vecintrin.h>
 
 volatile vector signed int vsi;
 volatile vector signed long long vsl;
 volatile vector unsigned int vui;

clang/test/CodeGen/builtins-systemz-zvector3.c

 // REQUIRES: systemz-registered-target
 // RUN: %clang_cc1 -target-cpu z15 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -target-cpu z15 -triple s390x-linux-gnu \
-// RUN: -O -fzvector -flax-vector-conversions=none \
+// RUN: -O2 -fzvector -flax-vector-conversions=none \
 // RUN: -Wall -Wno-unused -Werror -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 #include <vecintrin.h>
 
 volatile vector signed char vsc;
 volatile vector signed short vss;
 volatile vector signed int vsi;
 volatile vector signed long long vsl;

clang/test/CodeGen/fma-builtins-constrained.c

 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -O -emit-llvm -o - | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR --check-prefix=UNCONSTRAINED %s
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -ffp-exception-behavior=strict -O -emit-llvm -o - | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR --check-prefix=CONSTRAINED %s
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -O -S -o - | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM --check-prefix=CHECK-ASM-UNCONSTRAINED %s
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -O -ffp-exception-behavior=strict -S -o - | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM --check-prefix=CHECK-ASM-CONSTRAINED %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -O2 -emit-llvm -o - | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR --check-prefix=UNCONSTRAINED %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -ffp-exception-behavior=strict -O2 -emit-llvm -o - | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR --check-prefix=CONSTRAINED %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -O2 -S -o - | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM --check-prefix=CHECK-ASM-UNCONSTRAINED %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +fma -O2 -ffp-exception-behavior=strict -S -o - | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM --check-prefix=CHECK-ASM-CONSTRAINED %s
 
 #include <immintrin.h>
 
 __m128 test_mm_fmadd_ps(__m128 a, __m128 b, __m128 c) {
   // COMMON-LABEL: test_mm_fmadd_ps
   // UNCONSTRAINED: call <4 x float> @llvm.fma.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}})
   // CONSTRAINED: call <4 x float> @llvm.experimental.constrained.fma.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}, metadata !{{.*}})
   // CHECK-ASM: vfmadd213ps

clang/test/Driver/O.c

 // Test that we parse and translate the -O option correctly.
 
 // RUN: %clang -O -### %s 2>&1 | FileCheck -check-prefix=CHECK-O %s
-// CHECK-O: -O2
+// CHECK-O: -O1
 
 // RUN: %clang -O0 -### %s 2>&1 | FileCheck -check-prefix=CHECK-O0 %s
 // CHECK-O0: -O0
 
 // RUN: %clang -O1 -### %s 2>&1 | FileCheck -check-prefix=CHECK-O1 %s
 // CHECK-O1: -O1

clang/test/Driver/clang_f_opts.c

 // RUN: %clang -### -S -fvectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize -fvectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -fvectorize -fno-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -ftree-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-tree-vectorize -fvectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-tree-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -ftree-vectorize -fno-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
-// RUN: %clang -### -S -O %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
+// RUN: %clang -### -S -O %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -O2 %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -O1 -fvectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -Ofast %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -O0 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -O1 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // RUN: %clang -### -S -Oz %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
 // CHECK-VECTORIZE: "-vectorize-loops"
 // CHECK-NO-VECTORIZE-NOT: "-vectorize-loops"
 
 // RUN: %clang -### -S -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fslp-vectorize -fno-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s
 // RUN: %clang -### -S -ftree-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-tree-slp-vectorize -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-tree-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s
 // RUN: %clang -### -S -ftree-slp-vectorize -fno-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s
-// RUN: %clang -### -S -O %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
+// RUN: %clang -### -S -O %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s
 // RUN: %clang -### -S -O2 %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -Oz %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -O1 -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -Ofast %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s

clang/test/Driver/lto.c

 /// lld does not need LLVMgold.
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
 // RUN:   -fuse-ld=lld -flto -### 2>&1 | FileCheck --check-prefix=NO-LLVMGOLD %s
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
 // RUN:   -fuse-ld=gold -flto -fno-lto -### 2>&1 | FileCheck --check-prefix=NO-LLVMGOLD %s
 // NO-LLVMGOLD-NOT: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
-// RUN:   -fuse-ld=lld -flto -O -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN:   -fuse-ld=lld -flto -O -### 2>&1 | FileCheck --check-prefix=O1 %s
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
 // RUN:   -fuse-ld=lld -flto -O1 -### 2>&1 | FileCheck --check-prefix=O1 %s
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
 // RUN:   -fuse-ld=lld -flto -Og -### 2>&1 | FileCheck --check-prefix=O1 %s
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
 // RUN:   -fuse-ld=lld -flto -O2 -### 2>&1 | FileCheck --check-prefix=O2 %s
 // RUN: %clang -target x86_64-unknown-linux-gnu --sysroot %S/Inputs/basic_cross_linux_tree %s \
 // RUN:   -fuse-ld=lld -flto -Os -### 2>&1 | FileCheck --check-prefix=O2 %s