code review 41660045: runtime: Fix race detector checks to ignore KindNoPoint...

runtime: Fix race detector checks to ignore KindNoPointers bit
when comparing kinds.

[khr, 2013-12-31 18:25:57]

Hello dvyukov@google.com (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[dave_cheney.net, 2013-12-31 21:47:16]

Thanks for the quick fix. How about this to avoid introducing a new local
variable?

lucky(~/go/src/pkg/runtime) % hg diff hashmap.c 
diff -r 58deb42f814a src/pkg/runtime/hashmap.c
--- a/src/pkg/runtime/hashmap.c Wed Jan 01 07:46:00 2014 +1100
+++ b/src/pkg/runtime/hashmap.c Wed Jan 01 08:46:13 2014 +1100
@@ -998,7 +998,7 @@
 {
        if(raceenabled && h != nil) {
                runtime·racereadpc(h, runtime·getcallerpc(&t),
runtime·mapaccess1);
-               if(t->key->kind == KindArray || t->key->kind == KindStruct)
+               if((t->key->kind & ~KindNoPointers) == KindArray ||
(t->key->kind & ~KindNoPointers) == KindStruct)
                        runtime·racereadrangepc(ak, t->key->size,
runtime·getcallerpc(&t), runtime·mapaccess1);
                else
                        runtime·racereadpc(ak, runtime·getcallerpc(&t),
runtime·mapaccess1);

[dvyukov, 2014-01-01 11:56:43]

https://codereview.appspot.com/41660045/diff/40001/src/pkg/runtime/hashmap.c
File src/pkg/runtime/hashmap.c (right):

https://codereview.appspot.com/41660045/diff/40001/src/pkg/runtime/hashmap.c#...
src/pkg/runtime/hashmap.c:1003: kind = t->key->kind & ~KindNoPointers;
I think now it's time to do:

static void
racereadkv(byte *p, Type *t, uintptr callerpc, uintptr pc)
{
    uint8 kind;

    kind = t->kind & ~KindNoPointers;
    if(kind == KindArray || kind == KindStruct)
        runtime·racereadrangepc(p, t->size, callerpc, pc);
    else
        runtime·racereadpc(p, callerpc, pc);
}

racereadkv(ak, t->key, runtime·getcallerpc(&t), runtime·mapaccess1);

[khr1, 2014-01-04 05:27:37]

Dave, I had your version originally but it felt too verbose.  I've gone
Dmitry's route and added separate functions for this.  PTAL.


On Wed, Jan 1, 2014 at 3:56 AM, <dvyukov@google.com> wrote:

>
> https://codereview.appspot.com/41660045/diff/40001/src/
> pkg/runtime/hashmap.c
> File src/pkg/runtime/hashmap.c (right):
>
> https://codereview.appspot.com/41660045/diff/40001/src/
> pkg/runtime/hashmap.c#newcode1003
> src/pkg/runtime/hashmap.c:1003: kind = t->key->kind & ~KindNoPointers;
> I think now it's time to do:
>
> static void
> racereadkv(byte *p, Type *t, uintptr callerpc, uintptr pc)
> {
>     uint8 kind;
>
>     kind = t->kind & ~KindNoPointers;
>     if(kind == KindArray || kind == KindStruct)
>         runtime·racereadrangepc(p, t->size, callerpc, pc);
>     else
>         runtime·racereadpc(p, callerpc, pc);
> }
>
> racereadkv(ak, t->key, runtime·getcallerpc(&t), runtime·mapaccess1);
>
> https://codereview.appspot.com/41660045/
>

[dave_cheney.net, 2014-01-04 05:40:56]

LGTM. The Darwin race builder likes this, and so do I  

> On 4 Jan 2014, at 16:27, Keith Randall <khr@google.com> wrote:
> 
> Dave, I had your version originally but it felt too verbose.  I've gone
Dmitry's route and added separate functions for this.  PTAL.
> 
> 
>> On Wed, Jan 1, 2014 at 3:56 AM, <dvyukov@google.com> wrote:
>> 
>> https://codereview.appspot.com/41660045/diff/40001/src/pkg/runtime/hashmap.c
>> File src/pkg/runtime/hashmap.c (right):
>> 
>>
https://codereview.appspot.com/41660045/diff/40001/src/pkg/runtime/hashmap.c#...
>> src/pkg/runtime/hashmap.c:1003: kind = t->key->kind & ~KindNoPointers;
>> I think now it's time to do:
>> 
>> static void
>> racereadkv(byte *p, Type *t, uintptr callerpc, uintptr pc)
>> {
>>     uint8 kind;
>> 
>>     kind = t->kind & ~KindNoPointers;
>>     if(kind == KindArray || kind == KindStruct)
>>         runtime·racereadrangepc(p, t->size, callerpc, pc);
>>     else
>>         runtime·racereadpc(p, callerpc, pc);
>> }
>> 
>> racereadkv(ak, t->key, runtime·getcallerpc(&t), runtime·mapaccess1);
>> 
>> https://codereview.appspot.com/41660045/
> 

[dvyukov, 2014-01-04 16:29:33]

LGTM

[khr, 2014-01-04 16:43:25]

*** Submitted as https://code.google.com/p/go/source/detail?r=06e400cf4c93 ***

runtime: Fix race detector checks to ignore KindNoPointers bit
when comparing kinds.

R=dvyukov, dave, khr
CC=golang-codereviews
https://codereview.appspot.com/41660045