Closed Bug 944460 Opened 11 years ago Closed 5 years ago

Make sure MOZ_NEVER_INLINE is not available on clang

Categories

(Core :: MFBT, defect)

Product:
Core
Component:
MFBT
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INCOMPLETE

People

(Reporter: ehsan.akhgari, Unassigned)

Details

It doesn't seem to do anything anyway: $ cat test.cpp __attribute__((noinline)) int f(); int f() { return 42; } int main() { return f(); } $ clang++ -O3 -S -c -o test.s test.cpp $ cat test.s .section __TEXT,__text,regular,pure_instructions .globl __Z1fv .align 4, 0x90 __Z1fv: ## @_Z1fv .cfi_startproc ## BB#0: pushq %rbp Ltmp2: .cfi_def_cfa_offset 16 Ltmp3: .cfi_offset %rbp, -16 movq %rsp, %rbp Ltmp4: .cfi_def_cfa_register %rbp movl $42, %eax popq %rbp ret .cfi_endproc .globl _main .align 4, 0x90 _main: ## @main .cfi_startproc ## BB#0: pushq %rbp Ltmp7: .cfi_def_cfa_offset 16 Ltmp8: .cfi_offset %rbp, -16 movq %rsp, %rbp Ltmp9: .cfi_def_cfa_register %rbp movl $42, %eax popq %rbp ret .cfi_endproc .subsections_via_symbols
OK, Benoit and I did an experiment. gcc 4.6 doesn't honor __attribute__((noinline)) no matter what we did.
Shouldn't this bug be renamed to "remove MOZ_NEVER_INLINE" ?
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #1) > OK, Benoit and I did an experiment. gcc 4.6 doesn't honor > __attribute__((noinline)) no matter what we did. I don't know what you're doing, but gcc 4.6 honors it for me.
Well, GCC 4.6.2 doesn't honor it for me. bjacob:/hack/mozilla-central$ g++ -O2 -c ehsan.cpp -o e && nm -C ./e ehsan.cpp:2:39: warning: inline function ‘void foo2()’ given attribute noinline [enabled by default] ehsan.cpp:6:13: warning: inline function ‘void foo6()’ given attribute noinline [enabled by default] 0000000000000000 T foo1() 0000000000000010 T foo3() 0000000000000020 T foo5() 0000000000000030 T ehsan() bjacob:/hack/mozilla-central$ cat ehsan.cpp __attribute__((noinline)) void foo1(); __attribute__((noinline)) inline void foo2(); void foo3(); inline void foo4(); void foo5() __attribute__((noinline)); inline void foo6() __attribute__((noinline)); void foo1() {} void foo2() {} void foo3() {} void foo4() {} void foo5() {} void foo6() {} void ehsan() { foo1(); foo2(); foo3(); foo4(); foo5(); foo6(); }bjacob:/hack/mozilla-central$ g++ -O2 -c ehsan.cpp -o e ehsan.cpp:2:39: warning: inline function ‘void foo2()’ given attribute noinline [enabled by default] ehsan.cpp:6:13: warning: inline function ‘void foo6()’ given attribute noinline [enabled by default] bjacob:/hack/mozilla-central$ g++ --version g++-4.6.real (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 Copyright (C) 2011 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. bjacob:/hack/mozilla-central$ nm -C ./e 0000000000000000 T foo1() 0000000000000010 T foo3() 0000000000000020 T foo5() 0000000000000030 T ehsan()
Oh wait.... all what this shows is that inline overrides the __attribute__((noinline)).
Also, as soon as the function body is non-empty, __attribute__((noinline)) takes even even on inline functions: bjacob:/hack/mozilla-central$ cat ehsan.cpp #include <iostream> __attribute__((noinline)) void foo1(); __attribute__((noinline)) inline void foo2(); void foo3(); inline void foo4(); void foo5() __attribute__((noinline)); inline void foo6() __attribute__((noinline)); void foo1() {std::cout << std::endl;} void foo2() {std::cout << std::endl;} void foo3() {std::cout << std::endl;} void foo4() {std::cout << std::endl;} void foo5() {std::cout << std::endl;} void foo6() {std::cout << std::endl;} int main() { foo1(); foo2(); foo3(); foo4(); foo5(); foo6(); } bjacob:/hack/mozilla-central$ g++ -O3 ehsan.cpp -o e && nm -C ./e | grep foo ehsan.cpp:4:39: warning: inline function ‘void foo2()’ given attribute noinline [enabled by default] ehsan.cpp:8:13: warning: inline function ‘void foo6()’ given attribute noinline [enabled by default] 0000000000400a80 t _GLOBAL__sub_I__Z4foo1v 0000000000400870 T foo1() 0000000000400990 W foo2() 00000000004008d0 T foo3() 0000000000400930 T foo5() 00000000004009f0 W foo6()
(In reply to Benoit Jacob [:bjacob] from comment #5) > Oh wait.... all what this shows is that inline overrides the > __attribute__((noinline)). So, there are several things. - nm -C is not the right test, because the compiler can inline *and* export the symbol. - there are *other* reasons than inlining for a function call to be removed by the compiler, and those empty functions are one. This is what the noinline gcc documentation says: noinline This function attribute prevents a function from being considered for inlining. If the function does not have side-effects, there are optimizations other than inlining that cause function calls to be optimized away, although the function call is live. To keep such calls from being optimized away, put asm (""); (see Extended Asm) in the called function, to serve as a special side-effect.
Both clang and gcc do the same thing, as far as i can tell.
(In reply to Mike Hommey [:glandium] from comment #8) > Both clang and gcc do the same thing, as far as i can tell. well, not for comment 0.
Hmm, interesting. Bug 943839 is the real context here, and the function that was affected there is indeed empty...
If MOZ_NEVER_INLINE can be made to work, it is occasionally very useful. The main use I've seen has been fixing cases where the compiler makes a bad inlining choice that creates a humungo stack frame that limits our max recursion depth (e.g., the JS_NEVER_INLINE on Interpret).

Six years later, with clang now our primary compiler... I'm going to assume things are fine now.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.