Re: CVS commit: src/sys/sys

[Paul Goyette <paul%whooppee.com@localhost>]

On Sun, 8 Jul 2018, Christos Zoulas wrote:

> > > Committed By:	pgoyette
> > > Date:		Sun Jul  8 06:21:42 UTC 2018
> > >
> > > Modified Files:
> > > 	src/sys/sys: types.h
> > >
> > > Log Message:
> > > Use a different, type-insensitive idiom for CLR().
> > >
> > > As discussed on IRC and proposed by dholland@, the existing idiom is
> > > type-sensitive, and will likely fail silently when the flags variable
> > > is a 64-bit type.
> > >
> > > No functional change intended.  If anything breaks, it was probably
> > > already broken.
> >
> > This is much worse, since it now will evaluate the expression twice.
> > Please do not just change macros like this without a proper discussion
> > on the mailing lists.
>
> Yes, the double evaluation is a show-stopper, please revert.

A quick inspection shows that no additional code is generated, at least 
when using gcc.  For example:

kern/vfs_bio.c:

   #1037: cv_signal(&needbuffer_cv);

   0xffffffff80a3ce88 <+127>:   callq  0xffffffff80998ef5 <cv_signal>

   #1040: if (ISSET(bp->b_cflags, BC_WANTED))

   0xffffffff80a3ce8d <+132>:   mov    0xec(%rbx),%eax
   0xffffffff80a3ce93 <+138>:   test   $0x800000,%eax
   0xffffffff80a3ce98 <+143>:   je     0xffffffff80a3cea5 <brelsel+156>

   #1041: CLR(bp->b_cflags, BC_WANTED|BC_AGE);
   0xffffffff80a3ce9a <+145>:   and    $0xff7ffffe,%eax   <<<<
   0xffffffff80a3ce9f <+150>:   mov    %eax,0xec(%rbx)

The compiler seems perfectly happy optimizing this code without
generating any additional evaluations.

HOWEVER, I'll be happy to revert this.  Since it was dholland@ who
proposed this (on IRC), I will let him initiate any needed discussion
on the tech-kern list.


+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+