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

From: Mathieu Desnoyers
Date: Fri Jul 18 2008 - 17:12:21 EST


* H. Peter Anvin (hpa@xxxxxxxxx) 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/