svn commit: r190098 - in head/sys/sparc64: fhc sparc64

[Christoph Mallon]

Marius Strobl schrieb:
> On Sat, Mar 21, 2009 at 12:03:16PM +0100, Christoph Mallon wrote:
>> These variables are not redundant. Before the variables had clear names 
>> (like rid or nregs, nintr), now they are called i and j, which carries 
>> no semantics at all.
> 
> IMO the code is simple enough so that the variable names do not
> need to carry semantics for clearity, the fact that just two
> reused temporary variables are enough for the whole function
> underlines this.

The function is about 170 lines long, so allow me to disagree that it is 
"simple enough". Also the number of variables, which are live at once, 
is to my experience not a good measure for code complexity (A high 
number usually indicates high complexity, but a low number does not 
indicate low complexity).

>> Also j has its address taken AND you use this variable in several 
>> different contexts (replacement for rid, later as loop counter). This 
>> makes it impossible[0] for the compiler to do many useful optimisations 
>> like keeping the value of the variable in a register!
> 
> It's good to know that doing so may costs performance in general,
> as this is an bus attach function performance totally isn't an
> issue here though.

I'm equally concerned with code clarity. A reader has to figure out the 
def-use-chains when he tries to understand the code. Distinct variables 
can make this task easier (ideally every variable, except for loop 
counters, is const so there is only one point where it is assigned).
Also re-using address-taken variables in contexts where the address is 
not necessary usually results in larger machine code.
It's interesting to note that here the needs for a compiler to be able 
to produce "good" code and clarity of code for a human reader nicely 
correlate.

>> More local variables cost nothing - especially no stack space, if this 
>> is your concern - on any somewhat modern compiler. Use as many of them 
>> as you want for code clarity. Do not reuse variables, whose address is 
>> taken, because often it is impossible for the compiler to do any 
>> optimisations.
> 
> My motivation for using fewer local variables is that style(9)
> mandates to not define variables in nested scope and to sort
> them, so reusing them reduces code churn in the long term.
> IIRC you said elsewhere that you're unhappy with at least the
> former rule but you should work on getting style(9) then.

You are right, I disagree with quite some rules in style(9) and this 
particular rule I disagree with the most. But looking at the history, 
making any suggestions here to change it will only result in the biggest 
bikeshed ever built. Hm, seeing here that these rules encourage people 
to actively reduce code clarity, I'll contemplate about making a suggestion.

	Christoph