Skip to content

Commit

Permalink
[fix] fix inconsistency between bpc and c compiler about alignment.
Browse files Browse the repository at this point in the history 
- the alignment behavior of bpc now is compatible with c compiler.
- add 1 example in codegen.bp and add fix some expected size in testbench code.
- update bmmo protocol.
  • Loading branch information
@yyc12345
yyc12345 committed Aug 15, 2023
1 parent 4e9b325 commit 6d5b38c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 58 deletions.
6 changes: 0 additions & 6 deletions examples/bmmo.bp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ natural struct map {
uint8[16] md5;
int32 level;
}
narrow struct named_map {
string name;
map_type type;
uint8[16] md5;
uint32 level;
}
natural reliable msg level_finish_v2_msg {
gnsid player_id;
int32 points, lives, lifeBonus, levelBonus;
Expand Down
7 changes: 7 additions & 0 deletions examples/codegen.bp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ reliable natural msg codegen_test_align {
enum_primitive[3] static_big_primitive2;
}

reliable natural msg codegen_test_align2 {
int8 data1;
int8[15] data2;
int32 data3;
int8 data4;
}

reliable narrow msg codegen_test_manual_align {
uint8 single_primitive1;
uint32 single_primitive2;
Expand Down
101 changes: 50 additions & 51 deletions src/bpc_semantic_values.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,34 @@ uint32_t bpcsmtv_get_bt_size(BPCSMTV_BASIC_TYPE bt) {
return basic_type_sizeof[bt];
}

// A helper function to get variable size and alignment requirement
static void bpcsmtv_get_variable_type_size_and_align_req(BPCSMTV_VARIABLE* variable, uint32_t* tsize, uint32_t* talign_req, const char* hint_name) {
if (variable->variable_type->full_uncover_is_basic_type) {
// direct basic type, enum or alias
// check len
if (G_UNLIKELY(variable->variable_type->full_uncover_basic_type == BPCSMTV_BASIC_TYPE_STRING)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Get string as natural elements in accident when getting basic type alignof.");
}
size_t index = (size_t)variable->variable_type->full_uncover_basic_type;
if (G_UNLIKELY(index >= basic_type_len)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Element offset overflow when getting basic type alignof.");
}

if (tsize != NULL) *tsize = basic_type_sizeof[index];
if (talign_req != NULL) *talign_req = basic_type_alignof[index];
} else {
// struct
// look up its natural struct
BPCSMTV_PROTOCOL_BODY* entry = bpcsmtv_registery_identifier_get(variable->variable_type->type_data.custom_type);
if (G_UNLIKELY(entry == NULL)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Fail to get already defined data struct \"%s\" when analysing \"%s\".",
variable->variable_type->type_data.custom_type, hint_name);
}

if (tsize != NULL) *tsize = entry->node_data.struct_data->struct_modifier->struct_size;
if (talign_req != NULL) *talign_req = entry->node_data.struct_data->struct_modifier->struct_unit_size;
}
}
void bpcsmtv_setup_field_layout(GSList* variables, BPCSMTV_STRUCT_MODIFIER* modifier, const char* hint_name) {
// check data
bool can_be_natural = true;
Expand Down Expand Up @@ -470,7 +498,7 @@ void bpcsmtv_setup_field_layout(GSList* variables, BPCSMTV_STRUCT_MODIFIER* modi
bpcerr_warning(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE,
"Speficied field layout modifier \"natural\" is not suit for \"%s\". we change it to \"narrow\" forcely.", hint_name);

modifier->is_narrow = !can_be_natural;
modifier->is_narrow = true;
}
} else {
bpcerr_info(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "The field layout modifier of \"%s\" is not found, we assume it as \"%s\".",
Expand All @@ -491,74 +519,45 @@ void bpcsmtv_setup_field_layout(GSList* variables, BPCSMTV_STRUCT_MODIFIER* modi
for (cursor = variables; cursor != NULL; cursor = cursor->next) {
variable = (BPCSMTV_VARIABLE*)cursor->data;

if (variable->variable_type->full_uncover_is_basic_type) {
// direct basic type, enum or alias
// check len
if (G_UNLIKELY(variable->variable_type->full_uncover_basic_type == BPCSMTV_BASIC_TYPE_STRING)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Get string as natural elements in accident when getting basic type alignof.");
}
size_t gotten_alignof_offset = (size_t)variable->variable_type->full_uncover_basic_type;
if (G_UNLIKELY(gotten_alignof_offset >= basic_type_len)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Element offset overflow when getting basic type alignof.");
}

uint32_t gotten_alignof = basic_type_alignof[gotten_alignof_offset];
modifier->struct_unit_size = modifier->struct_unit_size >= gotten_alignof ?
modifier->struct_unit_size : gotten_alignof;
} else {
// struct
// look up its natural struct
BPCSMTV_PROTOCOL_BODY* entry = bpcsmtv_registery_identifier_get(variable->variable_type->type_data.custom_type);
if (G_UNLIKELY(entry == NULL)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Fail to get already defined data struct \"%s\" when analysing \"%s\".",
variable->variable_type->type_data.custom_type, hint_name);
}
// get align size
uint32_t align_size;
bpcsmtv_get_variable_type_size_and_align_req(variable, NULL, &align_size, hint_name);

BPCSMTV_STRUCT_MODIFIER* ref_modifier = entry->node_data.struct_data->struct_modifier;
// compare and get larger one.
modifier->struct_unit_size = modifier->struct_unit_size >= ref_modifier->struct_unit_size ?
modifier->struct_unit_size : ref_modifier->struct_unit_size;
}
// compare and get larger one.
modifier->struct_unit_size = modifier->struct_unit_size >= align_size ?
modifier->struct_unit_size : align_size;
}

// set align for each item
modifier->struct_size = UINT32_C(0);
uint32_t padding = UINT32_C(0), real_size = UINT32_C(0);
BPCSMTV_VARIABLE* nextvar;
for (cursor = variables; cursor != NULL; cursor = cursor->next) {
variable = (BPCSMTV_VARIABLE*)cursor->data;
nextvar = (cursor->next == NULL) ? NULL : ((BPCSMTV_VARIABLE*)cursor->next->data);

// calc real size first
// considering base type
if (variable->variable_type->full_uncover_is_basic_type) {
// direct basic type, enum or alias
if (G_UNLIKELY(variable->variable_type->full_uncover_basic_type == BPCSMTV_BASIC_TYPE_STRING)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Get string as natural elements in accident when getting basic type sizeof.");
}
size_t gotten_sizeof_offset = (size_t)variable->variable_type->full_uncover_basic_type;
if (G_UNLIKELY(gotten_sizeof_offset >= basic_type_len)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Element offset overflow when getting basic type sizeof.");
}

real_size = basic_type_sizeof[gotten_sizeof_offset];
// get this variable size
bpcsmtv_get_variable_type_size_and_align_req(variable, &real_size, NULL, hint_name);

// and get align boundary
uint32_t align_boundary;
if (nextvar == NULL) {
// last memeber should align with struct align
align_boundary = modifier->struct_unit_size;
} else {
// struct
// look up its natural struct
BPCSMTV_PROTOCOL_BODY* entry = bpcsmtv_registery_identifier_get(variable->variable_type->type_data.custom_type);
if (G_UNLIKELY(entry == NULL)) {
bpcerr_panic(BPCERR_ERROR_SOURCE_SEMANTIC_VALUE, "Fail to get already defined data struct \"%s\" when analysing \"%s\".",
variable->variable_type->type_data.custom_type, hint_name);
}

real_size = entry->node_data.struct_data->struct_modifier->struct_size;
// non-last memeber should pad blank for next variable required boundary
bpcsmtv_get_variable_type_size_and_align_req(nextvar, NULL, &align_boundary, hint_name);
}

// calc real size
// considering array
if (variable->variable_array->is_array) {
real_size *= variable->variable_array->static_array_len;
}

// calc padding
// Ref: https://en.wikipedia.org/wiki/Data_structure_alignment
padding = (modifier->struct_unit_size - (real_size & (modifier->struct_unit_size - UINT32_C(1)))) & (modifier->struct_unit_size - UINT32_C(1));
padding = (align_boundary - ((modifier->struct_size + real_size) % align_boundary)) % align_boundary;

// set align if necessary
// raise warning if there is a existed align
Expand Down
3 changes: 2 additions & 1 deletion testbenches/testbench.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,9 @@ def BenchmarkTest(vtypes: dict[str, dict], msgs: list[str]):
EXPECTED_STR_DLEN = len(DEFAULT_STR.encode('utf-8', errors='ignore')) + 4
EXPECTED_SIZE: dict[str, int] = {
'codegen_test_align': (1 + 3) + 4 + (1 * 3 + 1) + (4 * 3) + (1 + 3) + 4 + (1 * 3 + 1) + (4 * 3),
"codegen_test_align2": 1 + 15 + 4 + (1 + 3),
'codegen_test_manual_align': 1 + 4 + (1 + 3) + 4 + (EXPECTED_STR_DLEN + 3) + EXPECTED_STR_DLEN,
'codegen_test_natural': 4 + (4 * 3) + (4 * 5) + (1 + 3) + (1 * 3 + 1) + (1 * 5 + 3),
'codegen_test_natural': 4 + (4 * 3) + (4 * 5) + 1 + (1 * 3) + (1 * 5 + 3),
'codegen_test_natural_alternative': 4 + (4 * 3) + 4 + (4 * 3) + 4 + (4 * 3),
'codegen_test_narrow': 4 + (4 * 3) + (4 * DEFAULT_LIST_LEN + 4) +
EXPECTED_STR_DLEN + (EXPECTED_STR_DLEN * 3) + (EXPECTED_STR_DLEN * DEFAULT_LIST_LEN + 4) +
Expand Down

0 comments on commit 6d5b38c

Please sign in to comment.