569361 - need NO_INLINE procedure annotation/directive

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Felix S. Klock II [:pnkfelix, :fklock]]

Ed and Felix have independently encountered spots where a NO_INLINE directive for the compiler would be useful.

In gcc, the "NO_INLINE" directive would expand to "__attribute__ ((noinline))"; Felix does not yet know what the corresponding expansion should be on other compilers.

Ed found the directive useful to control the size of core routines; by keeping slow-paths out-of-line, he reduced register pressure (and thus the number of spills), and thus had tighter code in his fast paths.

Felix found the directive useful for writing garbage collection self-tests: the inlining of subroutines generating garbage objects ended up accidentally pinning the generated objects because of spilled references on the stack.  By forcing the garbage generation routine to be out-of-line, the spilled references were located on a stack frame that is subsequently popped, and so the garbage collector is able to properly identify the objects as unreachable.

Comment 1

[Lars T Hansen]

When you land this you should make a note in comments (in VMPI.h /and/ in all the implementations) that code should not depend on it being honored, probably.  We had some __attribute__((noinline)) code in the GC that required that, but it was removed.

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #1)
> When you land this you should make a note in comments (in VMPI.h /and/ in all
> the implementations) that code should not depend on it being honored, probably.
>  We had some __attribute__((noinline)) code in the GC that required that, but
> it was removed.

I agree.

But the issue of NO_INLINE being potentially unsupported does raise a question: if a client really *does* need to ensure that a function call is not inlined, such as in my self-test example, should that client then not even bother with NO_INLINE, and instead use other means, like making the call through a function pointer to avoid the inlining?

(or perhaps such clients should just take the middle ground of first using NO_INLINE, leaving a note about what alternative techniques such as fcn pts might be employed in the future if one encounters a target where NO_INLINE is unsupported?)

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #2)
> But the issue of NO_INLINE being potentially unsupported does raise a question:
> if a client really *does* need to ensure that a function call is not inlined,
> such as in my self-test example, should that client then not even bother with
> NO_INLINE, and instead use other means, like making the call through a function
> pointer to avoid the inlining?

(Bah, ignore this; the obvious answer is that nothing precluded the client from employing both techniques at the same time, so it would be best to include the NO_INLINE directive even when you think it might be ignored.)

Comment 4

[Steven Johnson]

Scope creep alert: probably waaay too much code churn to entertain this, but in hindsight, REALLY_INLINE might be better named "ALWAYS_INLINE"...

Comment 5

[Steven Johnson]

Fun fact: experimenting with GCC4.0 and /O3, if I have

file foo.h:
template<> class foo {
    void bar()
}

file foo-inlines.h:
template<> void foo::bar() { /*whatever*/ }

if I want bar() to be non-inlined, I must put the __attribute__((noinline)) declaration in foo.h, not in foo-inlines.h -- adding to foo-inlines.h as well has no apparent effect.

Comment 6

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #4)
> Scope creep alert: probably waaay too much code churn to entertain this, but in
> hindsight, REALLY_INLINE might be better named "ALWAYS_INLINE"...

Except it doesn't actually mean that, e.g. Mac OS X Debug builds don't.

Comment 7

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: add NO_INLINE directive

Comment 8

[Edwin Smith]

Comment on attachment 516313 [details] [diff] [review]
add NO_INLINE directive

I'd consider adding some documentation comments, whatever you feel you can say with authority.  see the pitfall in comment #5.  (is that only for template functions, or must NO_INLINE always go on the declaration/prototype?, unlike REALLY_INLINE, which is on the definition/body)

Comment 9

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #5)
> Fun fact: experimenting with GCC4.0 and /O3, if I have
> 
> file foo.h:
> template<> class foo {
>     void bar()
> }
> 
> file foo-inlines.h:
> template<> void foo::bar() { /*whatever*/ }
> 
> if I want bar() to be non-inlined, I must put the __attribute__((noinline))
> declaration in foo.h, not in foo-inlines.h -- adding to foo-inlines.h as well
> has no apparent effect.

Steven: could you elaborate on this point a touch, with a more concrete test case?

I am trying to recreate the result of your experiment (in order to figure out the best set of caveats to document, but I cannot figure out if you meant to write "template<> class foo { ... };", or if the "template<> class foo" was a placeholder for something like "template<typename T> class foo"

If its been too long and its too difficult to recreate the test context, just let me know.

Comment 10

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #9)
> (In reply to comment #5)
> > Fun fact: experimenting with GCC4.0 and /O3, if I have
> > 
> > file foo.h:
> > template<> class foo {
> >     void bar()
> > }
> > 
> > file foo-inlines.h:
> > template<> void foo::bar() { /*whatever*/ }
> > 
> > if I want bar() to be non-inlined, I must put the __attribute__((noinline))
> > declaration in foo.h, not in foo-inlines.h -- adding to foo-inlines.h as well
> > has no apparent effect.
> 
> Steven: could you elaborate on this point a touch, with a more concrete test
> case?

To illustrate what I mean: here is a concrete attempt to replicate the result of your experiment

% echo && echo "== file: foo.cpp ==" \
> && cat foo.cpp \
> && g++-4.0  -O3 -S foo.cpp \
> && echo "== portion of: foo.s ==" \
> && grep -B 10 -A 3 call foo.s 

== file: foo.cpp ==
template<typename T> struct foo { void bar(); };

template<typename T>
__attribute__((noinline))
void foo<T>::bar() { return; }

int main() { foo<int> local; local.bar(); }
== portion of: foo.s ==
_main:
LFB3:
	pushl	%ebp
LCFI2:
	movl	%esp, %ebp
LCFI3:
	subl	$40, %esp
LCFI4:
	leal	-9(%ebp), %eax
	movl	%eax, (%esp)
	call	L__ZN3fooIiE3barEv$stub
	xorl	%eax, %eax
	leave
	ret

Comment 11

[Tamarin Bot]

changeset: 6030:ab191cb3dea8
user:      Felix S Klock II <[e-mail]>
summary:   Bug 569361: add NO_INLINE directive to VMPI (r=edwsmith).

http://hg.mozilla.org/tamarin-redux/rev/ab191cb3dea8

Comment 12

[Steven Johnson]

(In reply to comment #9)
> If its been too long and its too difficult to recreate the test context, just
> let me know.

Sorry: it's been too long, don't remember. You probably know more about that than I do now.

Comment 13

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #12)
> (In reply to comment #9)
> > If its been too long and its too difficult to recreate the test context, just
> > let me know.
> 
> Sorry: it's been too long, don't remember. You probably know more about that
> than I do now.

No problem; I left a healthy amount of wiggle room in the documentation.