[SLOF] [PATCH 4/4] fbuffer: Implement RFILL as an accelerated primitive

Segher Boessenkool segher at kernel.crashing.org
Sun Aug 2 20:07:28 AEST 2015


On Sun, Aug 02, 2015 at 09:52:38AM +0200, Thomas Huth wrote:
> > "register" is useless.
> 
> ... apart from the fact that you can not take the address of a register
> variable with the "&" operator and thus gain a little bit more hope that
> the compiler puts the variable into a register.

Not really no.  Not at all, in fact.

> Apart from that, I know, the compiler can ignore this completely ... but
> it gives me a better feeling when I write it there ;-)

Better start using "auto" as well then :-)

> > You might need "unsigned" there though, or is "t" always unsigned already?
> 
> The used types are unsigned already, see slof/types.h.

"t" is a macro arg, I don't see all (future) callers :-)  But yeah,
I probably shouldn't worry.

> > You shouldn't need that (t *) cast.
> 
> Ok, should be fine for the current code - until somebody tries to use
> the macro with a different kind of dst variable.

Yes, but then if there is a problem, you are *hiding* the problem instead
of letting the compiler complain.

> >> +#define _FASTRFILL(dst, size, pat) \
> >> +	switch (((type_u)dst | size) & (sizeof(type_u)-1)) { \
> >> +		case 0:			_RFILL(dst, size, pat, type_u); break; \
> >> +		case sizeof(type_l):	_RFILL(dst, size, pat, type_l); break; \
> >> +		case sizeof(type_w):	_RFILL(dst, size, pat, type_w); break; \
> >> +		default:		_RFILL(dst, size, pat, type_c); break; \
> >> +	}
> > 
> > This doesn't handle case 6 as "w".
> 
> True ... actually, I've just copy-n-pasted this code from _FASTRMOVE
> which also lacks this problem!

Yeah :-)  The only cases you really care about are 0 and default anyway,
but handling 2 and not handling 6 is strange (and you could just write
the numbers, type_l is set up to be always exactly 4 bytes, etc.; everything
will fall apart if it isn't).

This whole RFILL, RMOVE business is iffy: for "real" I/O accesses (not just
uncached memory as you use it for) you really need to enforce the access
size, not for speed but for correctness.


Segher


More information about the SLOF mailing list