Re: [PATCH v1 1/4] iio: afe: rescale: Don't use ^ for booleans

['Andy Shevchenko' <andriy.shevchenko@linux.intel.com>]

On Fri, Dec 06, 2024 at 01:24:09PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 04 December 2024 01:33
> > 
> > There are two (non-critical) issues with the code. First of all,
> > the eXclusive OR is not defined for booleans, so boolean to integer
> > promotion is required, Second, the u32 variable is used to keep
> > boolean value, so boolean is converted implicitly to the integer.
> 
> Except there is no such thing as 'boolean' they are all integers.

I believe this is an exercise in linguistics as I'm not native speaker
but I am very well aware of the promotions to the integer values.

> And the compiler has to have some set of rules to handle the cases
> where the memory that hold the 'boolean' doesn't have the value 0 or 1.

No doubts.

...

> >  		 * If only one of the rescaler elements or the schan scale is
> >  		 * negative, the combined scale is negative.
> >  		 */
> > -		if (neg ^ ((rescale->numerator < 0) ^ (rescale->denominator < 0))) {
> > +		if (neg != (rescale->numerator < 0 || rescale->denominator < 0)) {
> 
> That is wrong, the || would also need to be !=.

Why do you think so? Maybe it's comment(s) that is(are) wrong?

> Which will all generate real pile of horrid code.
> (I think the x86 version will stun you.)

I think your remark is based on something, can you show the output to elaborate
what exactly becomes horrible in this case?

> I'm guessing that somewhere there is a:
> 	neg = value < 0;

Nope.

> Provided all the values are the same size (eg int/s32), in which case:
> 	neg = value;
> ...
> 	if ((neg ^ rescale->numerator ^ rescale->denominator) < 0)
> will be the desired test.

-- 
With Best Regards,
Andy Shevchenko