Re: [PATCH ftrace.git] Immedate Values Optimized Jump Fix

[Mathieu Desnoyers <compudj@krystal.dyndns.org>]

* 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