[U-Boot] [PATCH 3/3] MIPS: Abstract cache op loops with a macro

[Marek Vasut]

On 05/27/2016 04:48 PM, Paul Burton wrote:
> On Fri, May 27, 2016 at 04:36:21PM +0200, Marek Vasut wrote:
>>> The problem is that then both that function & its callers would need to
>>> know about the types of cache ops, and there'd need to be some mapping
>>> from flags to the actual cache op values (of which there'll be a couple
>>> more once L2 support is added).
>>
>> But the callers already know which ops they must invoke, right ?
>> You can just have enum {} for the bit definitions. It's all local
>> to this file, so I don't see a problem.
> 
> It's all local to the file, but it's still an extra level of abstraction
> that I don't see the value of. It just means that both callers & callee
> have to know about various cache ops, and I don't see the point in
> duplicating that knowledge. I think it's much cleaner to keep that
> knowledge in one place - the caller.

OK, I see what you mean.

>>> I think it's much simpler & cleaner to
>>> just go with the macro in this case - it keeps knowledge of which cache
>>> ops to use with the callers, and it ends up generating the same code as
>>> before (ie. the inner loop gets unrolled to just the relevant cache
>>> instructions).
>>
>> I would expect GCC to take care of inlining the code, but you can always
>> use the always_inline directive if that's your concern. Even cleaner way
>> might be to use CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED and compile the
>> cache file with -O2 .
> 
> It may well inline it, remove the flags indirection & generate the same
> code - I haven't tried it to find out. Making the code less clear with
> that abstraction, even if the compiler will come along & clean it back
> up, doesn't make much sense to me as someone who has to read the code.

It makes sense when you debug the code with GDB though. It's much easier
to locate lines if you don't use huge variadic macros.

> I understand the preference for functions over macros, but I think this
> is a case where the macro has strengths that a function cannot provide
> (ignoring perhaps varargs which would be even more messy).

What would you say to turning this into a variadic function then ?
It would avoid having the same knowledge in both caller and callee and
it would not be a macro, so it's a win-win. What do you think ?

> Thanks,
>     Paul
> 


-- 
Best regards,
Marek Vasut