Re: [PATCH] mtd: spi-nor: fix the wrong dummy value

[Marek Vasut <marex@denx.de>]

On Thursday, April 17, 2014 at 05:57:59 PM, Huang Shijie wrote:
> On Thu, Apr 17, 2014 at 05:55:07PM +0200, Gerhard Sittig wrote:
> > On Thu, 2014-04-17 at 21:41 +0800, Huang Shijie wrote:
> > > The disassemble code for "int dummy = 8; dummy /= 8;" is:
> > > --------------------------------------------------
> > > 
> > >     83a6:	2308      	movs	r3, #8
> > >     83a8:	607b      	str	r3, [r7, #4]
> > >     83aa:	687b      	ldr	r3, [r7, #4]
> > >     83ac:	1dda      	adds	r2, r3, #7
> > >     83ae:	2b00      	cmp	r3, #0
> > >     83b0:	bfb4      	ite	lt
> > >     83b2:	4613      	movlt	r3, r2
> > >     83b4:	461b      	movge	r3, r3
> > >     83b6:	10db      	asrs	r3, r3, #3
> > >     83b8:	607b      	str	r3, [r7, #4]
> > > 
> > > --------------------------------------------------
> > > 
> > > The disassemble code for "int dummy = 8; dummy >>= 3;" is:
> > > --------------------------------------------------
> > > 
> > >     83a6:	2308      	movs	r3, #8
> > >     83a8:	607b      	str	r3, [r7, #4]
> > >     83aa:	687b      	ldr	r3, [r7, #4]
> > >     83ac:	10db      	asrs	r3, r3, #3
> > >     83ae:	607b      	str	r3, [r7, #4]
> > > 
> > > --------------------------------------------------
> > > 
> > > Obviously, the "dummy >>= 3" is faster then "dummy /= 8".
> > 
> > That is because of signedness.  Both forms of "/= 8" and ">>= 3"
> > should be identical to the compiler, and generate the same
> > output.  Compilers know that division by powers of two can be
> > done with a shift.
> > 
> > Signedness apparently makes a difference.  If you know that the
> > number of clocks always is non-negative, use appropriate data
> > types.  Or let the compiler carry out the correct instructions
> > for the very data type that was declared.  Pick one, don't
> > violate abstractions.
> > 
> > Counter example, matching the expectation:
> > 	unsigned int u_div(unsigned int v) {
> > 	
> > 		return v / 8;
> > 	
> > 	}
> > 	
> > 	unsigned int u_shift(unsigned int v) {
> > 	
> > 		return v >> 3;
> > 	
> > 	}
> > 	
> > 	0000001c <u_div>:
> > 	  1c:   e1a001a0        lsr     r0, r0, #3
> > 	  20:   e12fff1e        bx      lr
> > 	
> > 	00000024 <u_shift>:
> > 	  24:   e1a001a0        lsr     r0, r0, #3
> > 	  28:   e12fff1e        bx      lr
> > 
> > Anyway, source code should be written for humans, as it gets read
> > more often than written, and maintenance is hard enough already.
> > Try to come up with a text search pattern to catch both the 3 and
> > 8 values at the same time.  Or try to easily see how they are the
> > same when there is no comment.  Is the code path so hot that
> > single instructions count so badly, that the downsides should be
> > considered acceptable?
> 
> okay.
> 
> I will change to "dummy /= 8" in the next version.

My impression is that you also need to change the signedness of the data type.