[Rawstudio-dev] [PATCH 2 of 7] x86_port: Isolate completly
anders at brander.dk
Thu Sep 28 21:03:54 CEST 2006
On Thu, 2006-09-28 at 20:31 +0200, Edouard Gomez wrote:
> > I really don't like this. 'cpuflags' was created especially to avoid
> > polluting the code with #ifdefs.
> > Are you sure this is the best approach? (I'm not sure it isn't, but I
> > don't like it).
> What's exactly a review is for ! Let's review why i did this, maybe it'l
> turn up that it wasn't the best option... (i already know the answer
> :-), but i wanted to address that uglyness in a second round of patches)
> 1 - If you have a partial port, may be a x86_64 or a ppc+altivec code,
> then cpuflags might be initialized so optimized code can be called
> adequately. BUT then, you can't distinguish which function is
> optimized or not, and you end calling stub functions in the case a
> function hasn't been optimized. That was my experience yesterday
> when i hadn't port the (no)cms rendering pair of functions.
This was exactly what I saw, a partial port - and that is a problem with
stubs, but it's a very limited problem - limited to 32/64-bit i386.
> 2 - So the core problem is the way we call optimized functions. As
> they're just a stub until they get optimized... we should make they
> complete their contract. A poor minded solution would be to put a
> call to the unoptimized function in the stub so even if
> a port is incomplete, all functions do their work. But this is not
> a solution... if you port to more architecture you'll end with
> stubs, with optimized functions and lot of if blocks that will end
> protected by #define if you don't separate eg: _MMX..._SSE space
> from _ALTIVEC one, or a big switch for each and every optimized
> function in the code (though most of them would be empty because no
> assembler could deal with their code for completly different
> The solution to all our problems is called a function pointer !
That is exactly what I used in rs_render_select() as you have noticed.
> The rs_detect_cpu_features i introduced is part of that plan. Instead of
> setting a simple cpuflags var, it would initialize a set of function
> pointers according to what is available for each platform *and* cpu
> type. Then the only file having nasty #defines would be rawstudio.c
> (though it's even better to separate that function in its own file), the
> rest of the code just uses function pointers, never dealing with #define
> nor if/switch(cpuflags) blocks.
This sounds like the right thing to do. Feel free to move all this to a
> So i advise you to commit as is for the moment , then, i will fix with
> the proper solution in a future patch. Is this plan ok for you ?
This is very much okay with me. Please go ahead.
More information about the Rawstudio-dev