tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: grischka
Subject: Re: [Tinycc-devel] [PATCH] tcc: remove buggy nodata_wanted optimization
Date: Thu, 22 Feb 2018 22:57:15 +0100
User-agent: Thunderbird 2.0.0.23 (Windows/20090812)

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 <stdio.h>

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 <address@hidden>

---
 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.c        2018-02-22 01:42:11.000000000 +0100
+++ tinycc/tccgen.c     2018-02-22 18:32:33.000000000 +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 & 0xC0000000) /* only static data 
output */
+#define STATIC_DATA_WANTED (nocode_wanted & 0x80000000) /* 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(&vtop->type, &align);
-            if (NODATA_WANTED)
-                size = 0, align = 1;
             offset = section_add(data_section, size, align);
             vpush_ref(&vtop->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(&type, 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, &align);
        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<nb;i++) {
                             if (tok == TOK_STR)
@@ -6830,11 +6819,11 @@ static void decl_initializer_alloc(CType
     Sym *sym = NULL;
     int saved_nocode_wanted = nocode_wanted;
 #ifdef CONFIG_TCC_BCHECK
-    int bcheck = tcc_state->do_bounds_check && !NODATA_WANTED;
+    int bcheck = tcc_state->do_bounds_check;
 #endif
if (type->t & VT_STATIC)
-        nocode_wanted |= NODATA_WANTED ? 0x40000000 : 0x80000000;
+        nocode_wanted |= 0x80000000;
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 CONFIG_TCC_BCHECK
@@ -7009,9 +6995,6 @@ static void decl_initializer_alloc(CType
     if (type->t & VT_VLA) {
         int a;
- if (NODATA_WANTED)
-            goto no_alloc;
-
         /* save current stack pointer */
         if (vlas_in_scope == 0) {
             if (vla_sp_root_loc == -1)
Index: tinycc/tests/tests2/96_nodata_wanted.c
===================================================================
--- tinycc.orig/tests/tests2/96_nodata_wanted.c 2018-02-22 01:42:11.000000000 
+0100
+++ tinycc/tests/tests2/96_nodata_wanted.c      2018-02-22 01:42:11.000000000 
+0100
@@ -25,60 +25,4 @@ void foo() {
     short w = &foo; /* 2 cast warnings */
 }
-#elif defined test_data_suppression_off || defined test_data_suppression_on
-
-#if defined test_data_suppression_on
-# define SKIP 1
-#else
-# define SKIP 0
-#endif
-
-#include <stdio.h>
-/* some gcc headers #define __attribute__ to empty if it's not gcc */
-#undef __attribute__
-
-int main()
-{
-    __label__ ts0, te0, ts1, te1;
-    int tl, dl;
-
-    static char ds0 = 0;
-    static char de0 = 0;
-    /* get reference size of empty jmp */
-ts0:;
-    if (!SKIP) {}
-te0:;
-    dl = -(&de0 - &ds0);
-    tl = -(&&te0 - &&ts0);
-
-    /* test data and code suppression */
-    static char ds1 = 0;
-ts1:;
-    if (!SKIP) {
-        static void *p = (void*)&main;
-        static char cc[] = "static string";
-        static double d = 8.0;
-
-        static struct __attribute__((packed)) {
-            unsigned x : 12;
-            unsigned char y : 7;
-            unsigned z : 28, a: 4, b: 5;
-        } s = { 0x333,0x44,0x555555,6,7 };
-
-        printf("data:\n");
-        printf("  %d - %.1f - %.1f - %s - %s\n",
-            sizeof 8.0, 8.0, d, __FUNCTION__, cc);
-        printf("  %x %x %x %x %x\n",
-            s.x, s.y, s.z, s.a, s.b);
-    }
-te1:;
-    static char de1 = 0;
-
-    dl += &de1 - &ds1;
-    tl += &&te1 - &&ts1;
-    printf("size of data/text:\n  %s/%s\n",
-        dl ? "non-zero":"zero", tl ? "non-zero":"zero");
-    /*printf("# %d/%d\n", dl, tl);*/
-}
-
 #endif
Index: tinycc/tests/tests2/96_nodata_wanted.expect
===================================================================
--- tinycc.orig/tests/tests2/96_nodata_wanted.expect    2018-02-22 
01:42:11.000000000 +0100
+++ tinycc/tests/tests2/96_nodata_wanted.expect 2018-02-22 01:42:11.000000000 
+0100
@@ -10,14 +10,3 @@
 [test_local_data_noerror]
 96_nodata_wanted.c:25: warning: assignment makes integer from pointer without 
a cast
 96_nodata_wanted.c:25: warning: nonportable conversion from pointer to 
char/short
-
-[test_data_suppression_off]
-data:
-  8 - 8.0 - 8.0 - main - static string
-  333 44 555555 6 7
-size of data/text:
-  non-zero/non-zero
-
-[test_data_suppression_on]
-size of data/text:
-  zero/zero

_______________________________________________
Tinycc-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/tinycc-devel





reply via email to

[Prev in Thread] Current Thread [Next in Thread]