Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reflect: move binary flag into map type #4369

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

reflect: move binary flag into map type #4369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions compiler/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
)
case *types.Map:
typeFieldTypes = append(typeFieldTypes,
types.NewVar(token.NoPos, nil, "extraFlags", types.Typ[types.Uint8]),
types.NewVar(token.NoPos, nil, "numMethods", types.Typ[types.Uint16]),
types.NewVar(token.NoPos, nil, "ptrTo", types.Typ[types.UnsafePointer]),
types.NewVar(token.NoPos, nil, "elementType", types.Typ[types.UnsafePointer]),
Expand Down Expand Up @@ -273,10 +274,6 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
metabyte |= 1 << 6
}

if hashmapIsBinaryKey(typ) {
metabyte |= 1 << 7
}

switch typ := typ.(type) {
case *types.Basic:
typeFields = []llvm.Value{c.getTypeCode(types.NewPointer(typ))}
Expand Down Expand Up @@ -333,7 +330,12 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
c.getTypeCode(types.NewSlice(typ.Elem())), // slicePtr
}
case *types.Map:
var extraFlags uint8
if hashmapIsBinaryKey(typ.Key()) {
extraFlags |= 1 // extraFlagIsBinaryKey
}
typeFields = []llvm.Value{
llvm.ConstInt(c.ctx.Int8Type(), uint64(extraFlags), false),
llvm.ConstInt(c.ctx.Int16Type(), 0, false), // numMethods
c.getTypeCode(types.NewPointer(typ)), // ptrTo
c.getTypeCode(typ.Elem()), // elem
Expand Down
2 changes: 1 addition & 1 deletion compiler/testdata/gc.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ target triple = "wasm32-unknown-wasi"
@"runtime/gc.layout:62-2000000000000001" = linkonce_odr unnamed_addr constant { i32, [8 x i8] } { i32 62, [8 x i8] c"\01\00\00\00\00\00\00 " }
@"runtime/gc.layout:62-0001" = linkonce_odr unnamed_addr constant { i32, [8 x i8] } { i32 62, [8 x i8] c"\01\00\00\00\00\00\00\00" }
@"reflect/types.type:basic:complex128" = linkonce_odr constant { i8, ptr } { i8 80, ptr @"reflect/types.type:pointer:basic:complex128" }, align 4
@"reflect/types.type:pointer:basic:complex128" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:basic:complex128" }, align 4
@"reflect/types.type:pointer:basic:complex128" = linkonce_odr constant { i8, i16, ptr } { i8 85, i16 0, ptr @"reflect/types.type:basic:complex128" }, align 4

; Function Attrs: allockind("alloc,zeroed") allocsize(0)
declare noalias nonnull ptr @runtime.alloc(i32, ptr, ptr) #0
Expand Down
10 changes: 5 additions & 5 deletions compiler/testdata/interface.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ target triple = "wasm32-unknown-wasi"
%runtime._interface = type { ptr, ptr }
%runtime._string = type { ptr, i32 }

@"reflect/types.type:basic:int" = linkonce_odr constant { i8, ptr } { i8 -62, ptr @"reflect/types.type:pointer:basic:int" }, align 4
@"reflect/types.type:pointer:basic:int" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:basic:int" }, align 4
@"reflect/types.type:pointer:named:error" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:named:error" }, align 4
@"reflect/types.type:basic:int" = linkonce_odr constant { i8, ptr } { i8 66, ptr @"reflect/types.type:pointer:basic:int" }, align 4
@"reflect/types.type:pointer:basic:int" = linkonce_odr constant { i8, i16, ptr } { i8 85, i16 0, ptr @"reflect/types.type:basic:int" }, align 4
@"reflect/types.type:pointer:named:error" = linkonce_odr constant { i8, i16, ptr } { i8 85, i16 0, ptr @"reflect/types.type:named:error" }, align 4
@"reflect/types.type:named:error" = linkonce_odr constant { i8, i16, ptr, ptr, ptr, [7 x i8] } { i8 116, i16 1, ptr @"reflect/types.type:pointer:named:error", ptr @"reflect/types.type:interface:{Error:func:{}{basic:string}}", ptr @"reflect/types.type.pkgpath.empty", [7 x i8] c".error\00" }, align 4
@"reflect/types.type.pkgpath.empty" = linkonce_odr unnamed_addr constant [1 x i8] zeroinitializer, align 1
@"reflect/types.type:interface:{Error:func:{}{basic:string}}" = linkonce_odr constant { i8, ptr } { i8 84, ptr @"reflect/types.type:pointer:interface:{Error:func:{}{basic:string}}" }, align 4
@"reflect/types.type:pointer:interface:{Error:func:{}{basic:string}}" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:interface:{Error:func:{}{basic:string}}" }, align 4
@"reflect/types.type:pointer:interface:{String:func:{}{basic:string}}" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:interface:{String:func:{}{basic:string}}" }, align 4
@"reflect/types.type:pointer:interface:{Error:func:{}{basic:string}}" = linkonce_odr constant { i8, i16, ptr } { i8 85, i16 0, ptr @"reflect/types.type:interface:{Error:func:{}{basic:string}}" }, align 4
@"reflect/types.type:pointer:interface:{String:func:{}{basic:string}}" = linkonce_odr constant { i8, i16, ptr } { i8 85, i16 0, ptr @"reflect/types.type:interface:{String:func:{}{basic:string}}" }, align 4
@"reflect/types.type:interface:{String:func:{}{basic:string}}" = linkonce_odr constant { i8, ptr } { i8 84, ptr @"reflect/types.type:pointer:interface:{String:func:{}{basic:string}}" }, align 4
@"reflect/types.typeid:basic:int" = external constant i8

Expand Down
31 changes: 19 additions & 12 deletions src/reflect/type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@
// elem *typeStruct // element type of the array
// arrayLen uintptr // length of the array (this is part of the type)
// slicePtr *typeStruct // pointer to []T type
// - map types (this is still missing the key and element types)
// - map types
// meta uint8
// extraMeta uint8
// nmethods uint16 (0)
// ptrTo *typeStruct
// elem *typeStruct
Expand Down Expand Up @@ -396,10 +397,14 @@ type Type interface {

// Constants for the 'meta' byte.
const (
kindMask = 31 // mask to apply to the meta byte to get the Kind value
flagNamed = 32 // flag that is set if this is a named type
flagComparable = 64 // flag that is set if this type is comparable
flagIsBinary = 128 // flag that is set if this type uses the hashmap binary algorithm
kindMask = 31 // mask to apply to the meta byte to get the Kind value
flagNamed = 32 // flag that is set if this is a named type
flagComparable = 64 // flag that is set if this type is comparable
)

// Constants for the 'extraFlags' byte in the map type.
const (
extraFlagIsBinaryKey = 1 // flag that is set if this type uses the hashmap binary algorithm
)

// The base type struct. All type structs start with this.
Expand Down Expand Up @@ -433,10 +438,11 @@ type arrayType struct {

type mapType struct {
rawType
numMethod uint16
ptrTo *rawType
elem *rawType
key *rawType
extraFlags uint8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the other types that have an elem pointer match up with the elemType base struct (I suppose I could have embedded it, but I didn't ...). Can you add the extraFlags field there as well as to the other elemType structs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think you are right. This would be a problem if the struct doesn't have any padding (like on AVR - not a big platform but still kind of a lurking bug). I'll look into a fix.

numMethod uint16
ptrTo *rawType
elem *rawType
key *rawType
}

type namedType struct {
Expand Down Expand Up @@ -980,9 +986,10 @@ func (t *rawType) Comparable() bool {
return (t.meta & flagComparable) == flagComparable
}

// isBinary returns if the hashmapAlgorithmBinary functions can be used on this type
func (t *rawType) isBinary() bool {
return (t.meta & flagIsBinary) == flagIsBinary
// isBinaryKey returns if the hashmapAlgorithmBinary functions can be used on
// the key type of this map.
func (t *mapType) isBinaryKey() bool {
return t.extraFlags&extraFlagIsBinaryKey != 0
}

func (t *rawType) ChanDir() ChanDir {
Expand Down
19 changes: 12 additions & 7 deletions src/reflect/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,9 +947,10 @@ func (v Value) MapKeys() []Value {
k := New(v.typecode.Key())
e := New(v.typecode.Elem())

typecode := (*mapType)(unsafe.Pointer((v.typecode.underlying())))
keyType := v.typecode.key()
keyTypeIsEmptyInterface := keyType.Kind() == Interface && keyType.NumMethod() == 0
shouldUnpackInterface := !keyTypeIsEmptyInterface && keyType.Kind() != String && !keyType.isBinary()
shouldUnpackInterface := !keyTypeIsEmptyInterface && keyType.Kind() != String && !typecode.isBinaryKey()

for hashmapNext(v.pointer(), it, k.value, e.value) {
if shouldUnpackInterface {
Expand Down Expand Up @@ -979,6 +980,7 @@ func (v Value) MapIndex(key Value) Value {
panic(&ValueError{Method: "MapIndex", Kind: v.Kind()})
}

typecode := (*mapType)(unsafe.Pointer(v.typecode.underlying()))
vkey := v.typecode.key()

// compare key type with actual key type of map
Expand All @@ -997,7 +999,7 @@ func (v Value) MapIndex(key Value) Value {
return Value{}
}
return elem.Elem()
} else if vkey.isBinary() {
} else if typecode.isBinaryKey() {
var keyptr unsafe.Pointer
if key.isIndirect() || key.typecode.Size() > unsafe.Sizeof(uintptr(0)) {
keyptr = key.value
Expand Down Expand Up @@ -1028,10 +1030,11 @@ func (v Value) MapRange() *MapIter {
panic(&ValueError{Method: "MapRange", Kind: v.Kind()})
}

typecode := (*mapType)(unsafe.Pointer(v.typecode.underlying()))
keyType := v.typecode.key()

keyTypeIsEmptyInterface := keyType.Kind() == Interface && keyType.NumMethod() == 0
shouldUnpackInterface := !keyTypeIsEmptyInterface && keyType.Kind() != String && !keyType.isBinary()
shouldUnpackInterface := !keyTypeIsEmptyInterface && keyType.Kind() != String && !typecode.isBinaryKey()

return &MapIter{
m: v,
Expand Down Expand Up @@ -1824,6 +1827,7 @@ func (v Value) SetMapIndex(key, elem Value) {
}

vkey := v.typecode.key()
typecode := (*mapType)(unsafe.Pointer(v.typecode.underlying()))

// compare key type with actual key type of map
if !key.typecode.AssignableTo(vkey) {
Expand Down Expand Up @@ -1859,7 +1863,7 @@ func (v Value) SetMapIndex(key, elem Value) {
hashmapStringSet(v.pointer(), *(*string)(key.value), elemptr)
}

} else if key.typecode.isBinary() {
} else if typecode.isBinaryKey() {
var keyptr unsafe.Pointer
if key.isIndirect() || key.typecode.Size() > unsafe.Sizeof(uintptr(0)) {
keyptr = key.value
Expand Down Expand Up @@ -1965,14 +1969,15 @@ func MakeMapWithSize(typ Type, n int) Value {
panic("reflect.MakeMapWithSize: negative size hint")
}

key := typ.Key().(*rawType)
val := typ.Elem().(*rawType)
typecode := (*mapType)(unsafe.Pointer(typ.(*rawType).underlying()))
key := typecode.rawType.key()
val := typecode.rawType.elem()

var alg uint8

if key.Kind() == String {
alg = hashmapAlgorithmString
} else if key.isBinary() {
} else if typecode.isBinaryKey() {
alg = hashmapAlgorithmBinary
} else {
alg = hashmapAlgorithmInterface
Expand Down
Loading