Messages in this thread | | | Date | Mon, 29 Sep 2008 23:54:33 -0700 | From | Jeremy Fitzhardinge <> | Subject | Re: [PATCH] x86: do not allow to optimize flag_is_changeable_p() |
| |
Yinghai Lu wrote: > On Mon, Sep 29, 2008 at 11:14 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote: > >> Krzysztof Helt wrote: >> >>> From: Krzysztof Helt <krzysztof.h1@wp.pl> >>> >>> The flag_is_changeable_p() is used by >>> has_cpuid_p() which can return different results >>> in the code sequence below: >>> >>> if (!have_cpuid_p()) >>> identify_cpu_without_cpuid(c); >>> >>> /* cyrix could have cpuid enabled via c_identify()*/ >>> if (!have_cpuid_p()) >>> return; >>> >>> Otherwise, the gcc 3.4.6 optimizes these two calls >>> into one which make the code not working correctly. >>> Cyrix cpus have the CPUID instruction enabled but >>> it is not detected due to the gcc optimization. >>> Thus the ARR registers (mtrr like) are not detected >>> on such a cpu. >>> >>> >> If "asm volatile" changes the code and fixes the bug, it seems like >> you're making use of an undocumented - or at least non-portable - behaviour. >> >> Does adding a "memory" clobber also fix the problem? That would have >> better defined characteristics. >> >> > > how about > > if (!have_cpuid_p()) { > identify_cpu_without_cpuid(c); > > /* cyrix could have cpuid enabled via c_identify()*/ > if (!have_cpuid_p()) > return; > } >
That doesn't help, does it? If gcc thinks it can get away with evaluating have_cpuid_p() once, then that's the same as:
if (!have_cpuid_p()) { identify_cpu_without_cpuid(c);
return; }
even though identify_cpu_without_cpuid() can cause the cpu to suddenly start supporting cpuid.
The trouble is that flag_is_changeable_p() doesn't have any obvious global dependencies; it just takes a constant argument and returns a result. The asm() needs to be updated to have a "memory" constraint as a stand-in for the specific constraint of "cpu has switched into cpuid-supporting state".
J
| |