Date: Fri, 18 Jul 2008 17:12:05 -0400
From: Mathieu Desnoyers <compudj@krystal.dyndns.org>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>, Ingo Molnar <mingo@redhat.com>,
       Steven Rostedt <rostedt@goodmis.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH ftrace.git] Immedate Values Optimized Jump Fix
Message-ID: <20080718211205.GA5423@Krystal>
References: <20080717031721.GA24504@Krystal> <20080718161523.GY6875@elte.hu> <20080718183124.GA31768@Krystal> <4880F410.3090804@zytor.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
In-Reply-To: <4880F410.3090804@zytor.com>
User-Agent: Mutt/1.5.16 (2007-06-11)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2895
Lines: 67

* H. Peter Anvin (hpa@zytor.com) wrote:
> Mathieu Desnoyers wrote:
>>>
>>>  $ git-log-line linus.. arch/x86/kernel/immediate.c
>>>  ee563d6: immediate values: jump liveliness
>>>  e26875a: Immediate Values - Jump
>>>  3fc8d03: Immediate Values - x86 Optimization NMI and MCE support
>>>
>>> ... but the topic is stalled right now, due to hpa having had objections. 
>>> Have those concerns been resolved? (Peter Cc:-ed)
>>>
>>> i'd have applied this fix, but it does not apply. The first chunk seems 
>>> already be present (in a different form), the second chunk looks much 
>>> different.
>> Hrm, I've edited directly the immediate values: jump liveliness patches,
>> which explains why it does not apply. I'll try to unapply/reapply/fold
>> the patches and see what it gives.
>> Plus, I've noticed that the "Text Edit Lock" patches are not in the
>> immediates branch, thus it fails to compile. Immediate values depends on
>> the Text Edit Lock patches.
>
> My previous objection was that flow of control really does need to be 
> understood by the compiler, and I don't see how that could have been 
> resolved without involving gcc.
>
> I'm not opposed to static jump optimization in general, far from it, but 
> doing it behind the back of the compiler is fraught with peril, and even if 
> it can be made correct is going to generate bad enough code that I have to 
> question if it is worth the additional complexity.
>
> I definitely do not approve of the attempt to truncate liveliness by 
> putting a clobber after the if branch; there is still intervening code 
> generated by the C compiler which is going to cause some extremely hard to 
> debug problems at some point.
>

Hrm, I thought that by following the execution flow to both branches and
by looking at the code pattern found (load immediate, test, branch) and
by placing a constraint on the eax register to detect the liveliness
region of that variable we would be guaranteed that the compiler could
not re-use this variable outside of the pattern scope.

The generated code you are talking about will generate a different code
pattern, won't it (e.g. saving the registers on the stack before the
branch) ? And in this case, we fall-back on the standard immediate
values.

However, I agree that doing this without compiler support has been a
pain. One thing we could do while we wait for gcc support is to merge
conditional-branch based immediate values only, which are less complex,
and later on add the static jump feature when supported by the compiler.

Mathieu

> 	-hpa
>


-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/