Re: [Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization

2018-03-12 Thread Mikulas Patocka


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

2018-02-23 Thread Michael Matz

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

2018-02-23 Thread Michael Matz
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

2018-02-22 Thread grischka

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;
-}
-