Re: [Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization
On Fri, 23 Feb 2018, Michael Matz wrote: > Hi, > > On Thu, 22 Feb 2018, Mikulas Patocka wrote: > > > I think that that nodata_wanted misoptimization should be removed at all > > - in the C language, you can jump to any location in the function with > > goto or switch-case statement - thus reasoning such as "all the > > variables behind if (0) are unreachable" is incorrect. > > > > If you wanted to do that optimization correctly, you'd need to build > > control flow graph, and tcc is just too simple to do that. > > You're right that as implemented the non-allocation of variables is wrong. > But a full CFG isn't needed to fix this, the allocation merely needs to be > postponed to the first real use. Unfortunately that is somewhat > complicated when there are initializers (which matter only for static vars > in this case, but still). > > So, yeah, the easy fix is to remove it, but with more work the > optimization (or parts of it) could be retained, grischka? > > > Ciao, > Michael. I think tcc is too simple to do such optimizations as holding variables and allocating them when they are used. If I need an optimizing compiler, I use gcc, if I need something fast and simple, I use tcc. Mikulas ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization
Hi, On Thu, 22 Feb 2018, grischka wrote: However NODATA_WANTED does more than that, for example it also suppresses the "hello" with if (0) puts("hello"); Therefor I'd suggest a less radical change, such as: @@ -6916,7 +6916,7 @@ static void decl_initializer_alloc(CType *type, AttributeDef *ad, int r, align = 1; } -if (NODATA_WANTED) +if (!v && NODATA_WANTED) size = 0, align = 1; if ((r & VT_VALMASK) == VT_LOCAL) { Thus variable space will be allocated always but initializers are still suppressed. (Bypassing initializers with goto is not allowed). For statics it is. E.g. the following program is valid and must print 42: #include int main() { goto doit; if (0) { static int a = 42; doit: printf ("%d\n", a); } } But the idea of supressing only nameless entities is a good one I think. Ciao, Michael. ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization
Hi, On Thu, 22 Feb 2018, Mikulas Patocka wrote: > I think that that nodata_wanted misoptimization should be removed at all > - in the C language, you can jump to any location in the function with > goto or switch-case statement - thus reasoning such as "all the > variables behind if (0) are unreachable" is incorrect. > > If you wanted to do that optimization correctly, you'd need to build > control flow graph, and tcc is just too simple to do that. You're right that as implemented the non-allocation of variables is wrong. But a full CFG isn't needed to fix this, the allocation merely needs to be postponed to the first real use. Unfortunately that is somewhat complicated when there are initializers (which matter only for static vars in this case, but still). So, yeah, the easy fix is to remove it, but with more work the optimization (or parts of it) could be retained, grischka? Ciao, Michael. ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization
Mikulas Patocka wrote: Hi The commit 7f1ab9b1e111b9ea9261eec4b7c6fb0f42591eef ("tccgen: nodata_wanted") breaks tcc, it generates incorrect code where different local variables are aliasing each other. It can be tested with this program, if you compile it with broken tcc, it prints "2". #include int main(void) { goto there; if (0) { int a, b; there: a = 1; b = 2; printf("%d\n", a); } return 0; } I think that that nodata_wanted misoptimization should be removed at all - in the C language, you can jump to any location in the function with goto or switch-case statement - thus reasoning such as "all the variables behind if (0) are unreachable" is incorrect. I see, right. However NODATA_WANTED does more than that, for example it also suppresses the "hello" with if (0) puts("hello"); Therefor I'd suggest a less radical change, such as: @@ -6916,7 +6916,7 @@ static void decl_initializer_alloc(CType *type, AttributeDef *ad, int r, align = 1; } -if (NODATA_WANTED) +if (!v && NODATA_WANTED) size = 0, align = 1; if ((r & VT_VALMASK) == VT_LOCAL) { Thus variable space will be allocated always but initializers are still suppressed. (Bypassing initializers with goto is not allowed). Btw, this one might fix the float problem from your previous mail: @@ -882,9 +882,12 @@ void gfunc_call(int nb_args) /* movq %xmm0, j*8(%rsp) */ gen_offs_sp(0xd60f66, 0x100, arg*8); } else { -/* movaps %xmm0, %xmmN */ -o(0x280f); -o(0xc0 + (arg << 3)); +if (arg) { +save_reg(TREG_XMM0 + arg); +/* movaps %xmm0, %xmmN */ +o(0x280f); +o(0xc0 + (arg << 3)); +} d = arg_prepare_reg(arg); /* mov %xmm0, %rxx */ o(0x66); Thanks, -- grischka If you wanted to do that optimization correctly, you'd need to build control flow graph, and tcc is just too simple to do that. Mikulas Signed-off-by: Mikulas Patocka--- tccgen.c | 31 --- tests/tests2/96_nodata_wanted.c | 56 --- tests/tests2/96_nodata_wanted.expect | 11 -- 3 files changed, 7 insertions(+), 91 deletions(-) Index: tinycc/tccgen.c === --- tinycc.orig/tccgen.c2018-02-22 01:42:11.0 +0100 +++ tinycc/tccgen.c 2018-02-22 18:32:33.0 +0100 @@ -51,8 +51,7 @@ ST_DATA SValue __vstack[1+VSTACK_SIZE], ST_DATA int const_wanted; /* true if constant wanted */ ST_DATA int nocode_wanted; /* no code generation wanted */ -#define NODATA_WANTED (nocode_wanted > 0) /* no static data output wanted either */ -#define STATIC_DATA_WANTED (nocode_wanted & 0xC000) /* only static data output */ +#define STATIC_DATA_WANTED (nocode_wanted & 0x8000) /* only static data output */ ST_DATA int global_expr; /* true if compound literals must be allocated globally (used during initializers parsing */ ST_DATA CType func_vt; /* current function return type (used by return instruction) */ ST_DATA int func_var; /* true if current function is variadic (used by return instruction) */ @@ -1296,8 +1295,6 @@ ST_FUNC int gv(int rc) /* CPUs usually cannot use float constants, so we store them generically in data segment */ size = type_size(>type, ); -if (NODATA_WANTED) -size = 0, align = 1; offset = section_add(data_section, size, align); vpush_ref(>type, data_section, offset, size); vswap(); @@ -4662,10 +4659,8 @@ ST_FUNC void unary(void) type.t |= VT_ARRAY; type.ref->c = len; vpush_ref(, data_section, data_section->data_offset, len); -if (!NODATA_WANTED) { -ptr = section_ptr_add(data_section, len); -memcpy(ptr, funcname, len); -} +ptr = section_ptr_add(data_section, len); +memcpy(ptr, funcname, len); next(); } break; @@ -6423,7 +6418,7 @@ static int decl_designator(CType *type, vstore(); } vpop(); -} else if (!NODATA_WANTED) { +} else { c_end = c + nb_elems * elem_size; if (c_end > sec->data_allocated) section_realloc(sec, c_end); @@ -6467,11 +6462,6 @@ static void init_putv(CType *type, Secti ) tcc_error("initializer element is not computable at load time"); -if (NODATA_WANTED) { -vtop--; -return; -} -
[Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization
Hi The commit 7f1ab9b1e111b9ea9261eec4b7c6fb0f42591eef ("tccgen: nodata_wanted") breaks tcc, it generates incorrect code where different local variables are aliasing each other. It can be tested with this program, if you compile it with broken tcc, it prints "2". #include int main(void) { goto there; if (0) { int a, b; there: a = 1; b = 2; printf("%d\n", a); } return 0; } I think that that nodata_wanted misoptimization should be removed at all - in the C language, you can jump to any location in the function with goto or switch-case statement - thus reasoning such as "all the variables behind if (0) are unreachable" is incorrect. If you wanted to do that optimization correctly, you'd need to build control flow graph, and tcc is just too simple to do that. Mikulas Signed-off-by: Mikulas Patocka--- tccgen.c | 31 --- tests/tests2/96_nodata_wanted.c | 56 --- tests/tests2/96_nodata_wanted.expect | 11 -- 3 files changed, 7 insertions(+), 91 deletions(-) Index: tinycc/tccgen.c === --- tinycc.orig/tccgen.c2018-02-22 01:42:11.0 +0100 +++ tinycc/tccgen.c 2018-02-22 18:32:33.0 +0100 @@ -51,8 +51,7 @@ ST_DATA SValue __vstack[1+VSTACK_SIZE], ST_DATA int const_wanted; /* true if constant wanted */ ST_DATA int nocode_wanted; /* no code generation wanted */ -#define NODATA_WANTED (nocode_wanted > 0) /* no static data output wanted either */ -#define STATIC_DATA_WANTED (nocode_wanted & 0xC000) /* only static data output */ +#define STATIC_DATA_WANTED (nocode_wanted & 0x8000) /* only static data output */ ST_DATA int global_expr; /* true if compound literals must be allocated globally (used during initializers parsing */ ST_DATA CType func_vt; /* current function return type (used by return instruction) */ ST_DATA int func_var; /* true if current function is variadic (used by return instruction) */ @@ -1296,8 +1295,6 @@ ST_FUNC int gv(int rc) /* CPUs usually cannot use float constants, so we store them generically in data segment */ size = type_size(>type, ); -if (NODATA_WANTED) -size = 0, align = 1; offset = section_add(data_section, size, align); vpush_ref(>type, data_section, offset, size); vswap(); @@ -4662,10 +4659,8 @@ ST_FUNC void unary(void) type.t |= VT_ARRAY; type.ref->c = len; vpush_ref(, data_section, data_section->data_offset, len); -if (!NODATA_WANTED) { -ptr = section_ptr_add(data_section, len); -memcpy(ptr, funcname, len); -} +ptr = section_ptr_add(data_section, len); +memcpy(ptr, funcname, len); next(); } break; @@ -6423,7 +6418,7 @@ static int decl_designator(CType *type, vstore(); } vpop(); -} else if (!NODATA_WANTED) { +} else { c_end = c + nb_elems * elem_size; if (c_end > sec->data_allocated) section_realloc(sec, c_end); @@ -6467,11 +6462,6 @@ static void init_putv(CType *type, Secti ) tcc_error("initializer element is not computable at load time"); -if (NODATA_WANTED) { -vtop--; -return; -} - size = type_size(type, ); section_reserve(sec, c + size); ptr = sec->data + c; @@ -6711,8 +6701,7 @@ static void decl_initializer(CType *type string in global variable, we handle it specifically */ if (sec && tok == TOK_STR && size1 == 1) { -if (!NODATA_WANTED) -memcpy(sec->data + c + len, tokc.str.data, nb); +memcpy(sec->data + c + len, tokc.str.data, nb); } else { for(i=0;i do_bounds_check && !NODATA_WANTED; +int bcheck = tcc_state->do_bounds_check; #endif if (type->t & VT_STATIC) -nocode_wanted |= NODATA_WANTED ? 0x4000 : 0x8000; +nocode_wanted |= 0x8000; flexible_array = NULL; if ((type->t & VT_BTYPE) == VT_STRUCT) { @@ -6900,9 +6889,6 @@ static void decl_initializer_alloc(CType align = 1; } -if (NODATA_WANTED) -size = 0, align = 1; - if ((r & VT_VALMASK) == VT_LOCAL) { sec = NULL; #ifdef