Browse Source

Default struct member reordering for minimal size
Rule: largest members to smallest; if same size, order in source order

Ginger Bill 9 years ago
parent
commit
7509cdceb8
7 changed files with 66 additions and 35 deletions
  1. 1 1
      examples/basic.odin
  2. 3 2
      examples/demo.odin
  3. 9 6
      src/checker/entity.cpp
  4. 32 13
      src/checker/expr.cpp
  5. 4 2
      src/checker/type.cpp
  6. 7 4
      src/codegen/ssa.cpp
  7. 10 7
      src/string.cpp

+ 1 - 1
examples/basic.odin

@@ -6,7 +6,7 @@ print_string_to_buffer :: proc(buf: ^[]byte, s: string) {
 	// NOTE(bill): This is quite a hack
 	// TODO(bill): Should I allow the raw editing of a slice by exposing its
 	// internal members?
-	Raw_Bytes :: struct {
+	Raw_Bytes :: struct #ordered {
 		data: ^byte
 		len:  int
 		cap:  int

+ 3 - 2
examples/demo.odin

@@ -1,13 +1,14 @@
 #load "basic.odin"
 
 main :: proc() {
-	Vector3 :: struct {
+	Vector4 :: struct {
 		x: i8
 		y: i32
 		z: i16
+		w: i16
 	}
 
-	v := Vector3{1, 4, 9}
+	v := Vector4{1, 4, 9, 16}
 
 	t := type_info(v)
 

+ 9 - 6
src/checker/entity.cpp

@@ -41,11 +41,13 @@ struct Entity {
 	union {
 		struct { ExactValue value; } Constant;
 		struct {
-			b8 visited;   // Cycle detection
-			b8 used;      // Variable is used
-			b8 is_field;  // Is struct field
-			b8 anonymous; // Variable is an anonymous
-			b8 is_using;  // `using` variable
+			b8  visited;   // Cycle detection
+			b8  used;      // Variable is used
+			b8  anonymous; // Variable is an anonymous
+			b8  is_using;  // `using` variable
+
+			i32 field_index; // Order in source
+			b8  is_field;    // Is struct field
 		} Variable;
 		struct {} TypeName;
 		struct {
@@ -103,8 +105,9 @@ Entity *make_entity_param(gbAllocator a, Scope *scope, Token token, Type *type,
 	return entity;
 }
 
-Entity *make_entity_field(gbAllocator a, Scope *scope, Token token, Type *type, b32 is_anonymous) {
+Entity *make_entity_field(gbAllocator a, Scope *scope, Token token, Type *type, b32 is_anonymous, i32 field_index) {
 	Entity *entity = make_entity_variable(a, scope, token, type);
+	entity->Variable.field_index = field_index;
 	entity->Variable.is_field  = true;
 	entity->Variable.anonymous = cast(b8)is_anonymous;
 	return entity;

+ 32 - 13
src/checker/expr.cpp

@@ -357,14 +357,15 @@ void check_fields(Checker *c, AstNode *node, AstNodeArray decls,
 				AstNode *name = vd->names[name_index];
 				Token name_token = name->Ident;
 
-				Entity *e = make_entity_field(c->allocator, c->context.scope, name_token, type, vd->is_using);
+				Entity *e = make_entity_field(c->allocator, c->context.scope, name_token, type, vd->is_using, cast(i32)field_index);
 				HashKey key = hash_string(name_token.string);
 				if (map_get(&entity_map, key) != NULL) {
 					// TODO(bill): Scope checking already checks the declaration
 					error(&c->error_collector, name_token, "`%.*s` is already declared in this type", LIT(name_token.string));
 				} else {
 					map_set(&entity_map, key, e);
-					fields[field_index++] = e;
+					fields[field_index] = e;
+					field_index++;
 					add_entity(c, c->context.scope, name, e);
 				}
 				add_entity_use(&c->info, name, e);
@@ -391,13 +392,19 @@ gb_global BaseTypeSizes __checker_sizes = {};
 gb_global gbAllocator   __checker_allocator = {};
 
 GB_COMPARE_PROC(cmp_struct_entity_size) {
-	// NOTE(bill): Biggest to smallest
+	// Rule: Biggest to smallest
+	// if same size, order by order in source
 	Entity *x = *(Entity **)a;
 	Entity *y = *(Entity **)b;
+	GB_ASSERT(x != NULL);
+	GB_ASSERT(y != NULL);
+	GB_ASSERT(x->kind == Entity_Variable);
+	GB_ASSERT(y->kind == Entity_Variable);
 	i64 xs = type_size_of(__checker_sizes, __checker_allocator, x->type);
 	i64 ys = type_size_of(__checker_sizes, __checker_allocator, y->type);
 	if (xs == ys) {
-		return string_cmp(&x->token.string, &y->token.string);
+		i32 diff = x->Variable.field_index - y->Variable.field_index;
+		return diff < 0 ? -1 : diff > 0;
 	}
 	return xs > ys ? -1 : xs < ys;
 }
@@ -430,24 +437,36 @@ void check_struct_type(Checker *c, Type *struct_type, AstNode *node, CycleChecke
 
 	check_fields(c, node, st->decls, fields, field_count, other_fields, other_field_count, cycle_checker, make_string("struct"));
 
+
+	struct_type->Record.struct_is_packed    = st->is_packed;
+	struct_type->Record.struct_is_ordered   = st->is_ordered;
+	struct_type->Record.fields              = fields;
+	struct_type->Record.fields_in_src_order = fields;
+	struct_type->Record.field_count         = field_count;
+	struct_type->Record.other_fields        = other_fields;
+	struct_type->Record.other_field_count   = other_field_count;
+
+
+
 	if (!st->is_packed && !st->is_ordered) {
 		// NOTE(bill): Reorder fields for reduced size/performance
+
+		Entity **reordered_fields = gb_alloc_array(c->allocator, Entity *, field_count);
+		for (isize i = 0; i < field_count; i++) {
+			reordered_fields[i] = struct_type->Record.fields_in_src_order[i];
+		}
+
 		// NOTE(bill): Hacky thing
 		__checker_sizes = c->sizes;
 		__checker_allocator = c->allocator;
-
 		// TODO(bill): Figure out rules more
 		// sorting rules
 		// compound literal order must match source not layout
-		// gb_sort_array(fields, field_count, cmp_struct_entity_size);
+		gb_sort_array(reordered_fields, field_count, cmp_struct_entity_size);
+
+		struct_type->Record.fields = reordered_fields;
 	}
 
-	struct_type->Record.struct_is_packed  = st->is_packed;
-	struct_type->Record.struct_is_ordered = st->is_ordered;
-	struct_type->Record.fields            = fields;
-	struct_type->Record.field_count       = field_count;
-	struct_type->Record.other_fields      = other_fields;
-	struct_type->Record.other_field_count = other_field_count;
 }
 
 void check_union_type(Checker *c, Type *union_type, AstNode *node, CycleChecker *cycle_checker) {
@@ -3097,7 +3116,7 @@ ExprKind check__expr_base(Checker *c, Operand *o, AstNode *node, Type *type_hint
 							      "Mixture of `field = value` and value elements in a structure literal is not allowed");
 							continue;
 						}
-						Entity *field = t->Record.fields[index];
+						Entity *field = t->Record.fields_in_src_order[index];
 
 						check_expr(c, o, elem);
 						if (index >= field_count) {

+ 4 - 2
src/checker/type.cpp

@@ -133,6 +133,8 @@ struct Type {
 			b32      struct_are_offsets_set;
 			b32      struct_is_packed;
 			b32      struct_is_ordered;
+			Entity **fields_in_src_order; // Entity_Variable
+
 
 			// Entity_Constant or Entity_TypeName
 			Entity **other_fields;
@@ -732,12 +734,12 @@ Selection lookup_field(Type *type_, String field_name, b32 is_type, Selection se
 			if (entity__any_type_info == NULL) {
 				Token token = {Token_Identifier};
 				token.string = type_info_str;
-				entity__any_type_info = make_entity_field(gb_heap_allocator(), NULL, token, t_type_info_ptr, false);
+				entity__any_type_info = make_entity_field(gb_heap_allocator(), NULL, token, t_type_info_ptr, false, 0);
 			}
 			if (entity__any_data == NULL) {
 				Token token = {Token_Identifier};
 				token.string = data_str;
-				entity__any_data = make_entity_field(gb_heap_allocator(), NULL, token, t_rawptr, false);
+				entity__any_data = make_entity_field(gb_heap_allocator(), NULL, token, t_rawptr, false, 1);
 			}
 
 			if (are_strings_equal(field_name, type_info_str)) {

+ 7 - 4
src/codegen/ssa.cpp

@@ -1875,26 +1875,29 @@ ssaValue *ssa_build_single_expr(ssaProcedure *proc, AstNode *expr, TypeAndValue
 			auto *st = &base_type->Record;
 			if (cl->elems != NULL && gb_array_count(cl->elems) > 0) {
 				gb_for_array(field_index, cl->elems) {
-					AstNode *elem = cl->elems[field_index];
+					isize index = field_index;
+					AstNode *elem = cl->elems[index];
 					ssaValue *field_expr = NULL;
 					Entity *field = NULL;
 
 					if (elem->kind == AstNode_FieldValue) {
 						ast_node(kv, FieldValue, elem);
 						Selection sel = lookup_field(base_type, kv->field->Ident.string, false);
-						field_index = sel.index[0];
+						index = sel.index[0];
 						field_expr = ssa_build_expr(proc, kv->value);
 					} else {
+						Selection sel = lookup_field(base_type, st->fields_in_src_order[field_index]->token.string, false);
+						index = sel.index[0];
 						field_expr = ssa_build_expr(proc, elem);
 					}
 
 					GB_ASSERT(ssa_type(field_expr)->kind != Type_Tuple);
 
-					field = st->fields[field_index];
+					field = st->fields[index];
 
 					Type *ft = field->type;
 					ssaValue *fv = ssa_emit_conv(proc, field_expr, ft);
-					ssaValue *gep = ssa_emit_struct_gep(proc, v, field_index, ft);
+					ssaValue *gep = ssa_emit_struct_gep(proc, v, index, ft);
 					ssa_emit_store(proc, gep, fv);
 				}
 			}

+ 10 - 7
src/string.cpp

@@ -43,10 +43,7 @@ gb_inline b32 are_strings_equal_ignore_case(String a, String b) {
 	return false;
 }
 
-GB_COMPARE_PROC(string_cmp) {
-	String x = *cast(String *)a;
-	String y = *cast(String *)b;
-
+int string_compare(String x, String y) {
 	if (x.len == y.len &&
 	    x.text == y.text) {
 		return 0;
@@ -65,9 +62,9 @@ GB_COMPARE_PROC(string_cmp) {
 	isize *lb = cast(isize *)y.text;
 
 	for (; curr_block < fast; curr_block++) {
-		if ((la[curr_block] ^ lb[curr_block]) != 0) {
+		if (la[curr_block] ^ lb[curr_block]) {
 			for (isize pos = curr_block*gb_size_of(isize); pos < n; pos++) {
-				if ((x.text[pos] ^ y.text[pos]) != 0) {
+				if (x.text[pos] ^ y.text[pos]) {
 					return cast(int)x.text[pos] - cast(int)y.text[pos];
 				}
 			}
@@ -75,7 +72,7 @@ GB_COMPARE_PROC(string_cmp) {
 	}
 
 	for (; offset < n; offset++) {
-		if ((x.text[offset] ^ y.text[offset]) != 0) {
+		if (x.text[offset] ^ y.text[offset]) {
 			return cast(int)x.text[offset] - cast(int)y.text[offset];
 		}
 	}
@@ -83,6 +80,12 @@ GB_COMPARE_PROC(string_cmp) {
 	return 0;
 }
 
+GB_COMPARE_PROC(string_cmp_proc) {
+	String x = *(String *)a;
+	String y = *(String *)b;
+	return string_compare(x, y);
+}
+
 
 gb_inline isize string_extension_position(String str) {
 	isize dot_pos = -1;