From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54376 invoked by alias); 8 Jul 2015 15:13:22 -0000 Mailing-List: contact jit-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: Sender: jit-owner@gcc.gnu.org Received: (qmail 54349 invoked by uid 89); 8 Jul 2015 15:13:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.98.7 on sourceware.org X-Virus-Found: No X-HELO: mx1.redhat.com Message-ID: <1436367926.24803.71.camel@surprise> Subject: Re: Filed PR jit/66812 for the code generation issue From: David Malcolm To: Dibyendu Majumdar Cc: jit@gcc.gnu.org Date: Thu, 01 Jan 2015 00:00:00 -0000 In-Reply-To: <1436365266.24803.65.camel@surprise> References: <1436365266.24803.65.camel@surprise> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-SW-Source: 2015-q3/txt/msg00031.txt.bz2 On Wed, 2015-07-08 at 10:21 -0400, David Malcolm wrote: > On Sat, 2015-07-04 at 16:58 +0100, Dibyendu Majumdar wrote: > > On 4 July 2015 at 14:20, Dibyendu Majumdar wrote: > > > On 4 July 2015 at 13:11, Dibyendu Majumdar wrote: > > >> Looks like in the failure case the code is being incorrectly > > >> optimized. I wonder if this is a manifestation of the get_address bug, > > >> perhaps the real fix will be better than the patch I am using. I will > > >> use the latest gcc 5 branch and see if that helps. > > >> > > > > > > Hi Dave, > > > > > > I am now using the latest gcc-5-branch from gcc github mirror. > > > Unfortunately the issue still persists. > > > > > > If set optimization level to 0 or 1, then it works ok, but at levels 2 > > > or 3 the break occurs. > > > > > > > Adding the -fno-strict-aliasing appears to resolve the issue with -O2 > > and -O3 but with this enabled the benchmarks are degraded. > > I've filed the bad code generation issue as: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66812 > > and I'm investigating it. Notes on investigation so far. With the fixes here https://gcc.gnu.org/ml/jit/2015-q3/msg00025.html and here: https://gcc.gnu.org/ml/jit/2015-q3/msg00028.html the reproducer from https://gcc.gnu.org/ml/jit/2015-q3/msg00012.html now correctly fails with: error: gcc_jit_context_new_field: unknown size for field "errorJmp" (type: struct ravi_lua_longjmp) Upon applying the equivalent of the fix from: https://github.com/dibyendumajumdar/ravi/commit/d65d2e68fbdcf211ed36deea05727f996ede8296 to the generator you provided, gcc_jit_context_compile completes. I don't know if it's generating bad code (given that I don't have Ravi itself running, just the reproducer). However, I tried comparing the bug_rdump.txt and bug_rdump_ok.txt reproducers, and tried adding -fno-strict-aliasing to the former. I turned on GCC_JIT_BOOL_OPTION_DUMP_EVERYTHING and started comparing the dumpfiles. The diffs stay easily comparable until fake.c.018t.ssa, when numbering differences between temporaries mean that a simple "diff" becomes unreadable. So I wrote a tool to try to alleviate this: https://github.com/davidmalcolm/gcc-dump-diff The tool tries to renumber temporaries in dumpfiles so that they are more consistent between runs, and then tries to do a textual diff of two dumpfiles. (note that it might renumber labels also) With this, the dumpfiles become comparable again. I see that when rerunning the bug_rdump.txt reproducer, the first big difference between the with and without -fno-strict-aliasing case happens at 035t.fre1. Note in the following how, without -fno-strict-aliasing, various statements and blocks are eliminated at 035t.fre1, which treats them as dead code. In particular, it seems to have optimized away OP_TEST_do_jmp_5_0 and OP_TEST_do_skip_5_0 (if I'm reading things right). My hunch at this time is that this optimization is being too aggressive... for some reason. I'll continue poking at this. --- with-fno-strict-aliasing/fake.c.035t.fre1 +++ without-fno-strict-aliasing/fake.c.035t.fre1 @@ -2,6 +2,15 @@ ;; Function ravif2 (ravif2, funcdef_no=0, decl_uid=364, cgraph_uid=0, symbol_order=0) +Merging blocks 8 and 9 +Removing basic block 10 +Removing basic block 11 +Removing basic block 13 +Removing basic block 12 +Removing basic block 22 +Removing basic block 17 +Merging blocks 8 and 14 +Merging blocks 8 and 15 ravif2 (struct ravi_lua_State * L) { struct ravi_TValue * base; @@ -20,7 +29,6 @@ comparison_0_8; comparison_0_9; iftmp.1_0; - iftmp.3_0; iftmp.5_0; struct ravi_CallInfo * _0; struct ravi_TValue * _0; @@ -28,31 +36,16 @@ struct ravi_Proto * _0; struct ravi_TValue * _1; signed long _0; + signed int _0; + signed int _1; + _0; + signed long _1; + signed int _1; + struct ravi_CallInfo * _2; + struct ravi_TValue * _0; struct ravi_Proto * _1; - struct ravi_TValue * _2; - signed int _0; - struct ravi_CallInfo * _2; - signed int _1; - signed int _2; - struct ravi_CallInfo * _3; signed int _3; - signed int _4; - _0; - struct ravi_CallInfo * _4; - struct ravi_Proto * _2; struct ravi_TValue * _3; - signed long _0; - struct ravi_Proto * _3; - struct ravi_TValue * _4; - signed int _0; - struct ravi_CallInfo * _5; - signed int _6; - signed int _7; - struct ravi_CallInfo * _6; - struct ravi_TValue * _0; - struct ravi_Proto * _4; - signed int _8; - struct ravi_TValue * _6; entry: _0 = L_0(D)->ci; @@ -66,14 +59,9 @@ _1 = _0->k; _0 = _1->value_.i; MEM[(struct ravi_TValue *)base_0 + 16B].value_.i = _0; - _1 = cl_0->p; - _2 = _1->k; - _0 = _2->tt_; + _0 = _1->tt_; MEM[(struct ravi_TValue *)base_0 + 16B].tt_ = _0; - _2 = L_0(D)->ci; - base_2 = _2->u.l.base; - _1 = MEM[(struct ravi_TValue *)base_1 + 16B].tt_; - _2 = MEM[(struct ravi_TValue *)base_1 + 16B].value_.b; + _1 = MEM[(struct ravi_TValue *)base_0 + 16B].value_.b; if (_1 == 0) goto ; else @@ -104,119 +92,56 @@ : # isfalse_0_4_0 = PHI <1(6), 0(7), 1(2)> - MEM[(struct ravi_TValue *)base_1 + 16B].value_.b = 0; - MEM[(struct ravi_TValue *)base_1 + 16B].tt_ = 1; - _3 = L_0(D)->ci; - base_3 = _3->u.l.base; - _3 = MEM[(struct ravi_TValue *)base_2 + 16B].tt_; - _4 = MEM[(struct ravi_TValue *)base_2 + 16B].value_.b; + MEM[(struct ravi_TValue *)base_0 + 16B].value_.b = 0; + MEM[(struct ravi_TValue *)base_0 + 16B].tt_ = 1; + _0 = 0; + _1 = _1->value_.i; + MEM[(struct ravi_TValue *)base_0 + 16B].value_.i = _1; + _1 = _1->tt_; + MEM[(struct ravi_TValue *)base_0 + 16B].tt_ = _1; if (_3 == 0) - goto ; + goto ; else goto ; : if (_3 == 1) + goto ; + else goto ; - else - goto ; : - if (_4 == 0) - goto ; - else - goto ; : + # iftmp.5_0 = PHI <1(9), 0(10)> + if (iftmp.5_0 != 0) + goto ; + else + goto ; : - # iftmp.3_0 = PHI <1(10), 0(11)> - if (iftmp.3_0 != 0) - goto ; - else - goto ; : - - : - # isfalse_0_10_0 = PHI <1(12), 0(13), 1(8)> - _0 = ~isfalse_0_10_0; - if (isfalse_0_10_0 != 0) - goto (OP_TEST_do_jmp_5_0); + # isfalse_0_16_0 = PHI <1(11), 0(12), 1(8)> + MEM[(struct ravi_TValue *)base_0 + 16B].tt_ = 1; + printf ("OP_RETURN(pc=%d) return %d args", 13, 1); + _2 = L_0(D)->ci; + base_2 = _2->u.l.base; + _0 = base_2 + 32; + L_0(D)->top = _0; + _1 = cl_0->p; + _3 = _1->sizep; + if (_4 > 0) + goto (OP_RETURN_if_sizep_gt_0_12_0); else - goto (OP_TEST_do_skip_5_0); - -OP_TEST_do_jmp_5_0: -jmp_9_0: - _4 = L_0(D)->ci; - base_4 = _4->u.l.base; - _2 = cl_0->p; - _3 = _2->k; - _0 = _3->value_.i; - MEM[(struct ravi_TValue *)base_3 + 16B].value_.i = _0; - _3 = cl_0->p; - _4 = _3->k; - _0 = _4->tt_; - MEM[(struct ravi_TValue *)base_3 + 16B].tt_ = _0; - _5 = L_0(D)->ci; - base_5 = _5->u.l.base; - _6 = MEM[(struct ravi_TValue *)base_4 + 16B].tt_; - _7 = MEM[(struct ravi_TValue *)base_4 + 16B].value_.b; - if (_5 == 0) - goto ; - else - goto ; - - : - if (_5 == 1) - goto ; - else - goto ; - - : - if (_6 == 0) - goto ; - else - goto ; - - : - - : - # iftmp.5_0 = PHI <1(17), 0(18)> - if (iftmp.5_0 != 0) - goto ; - else - goto ; - - : - - : - # isfalse_0_16_0 = PHI <1(19), 0(20), 1(15)> - MEM[(struct ravi_TValue *)base_4 + 16B].value_.b = 0; - MEM[(struct ravi_TValue *)base_4 + 16B].tt_ = 1; - printf ("OP_RETURN(pc=%d) return %d args", 13, 1); - _6 = L_0(D)->ci; - base_6 = _6->u.l.base; - _0 = base_6 + 32; - L_0(D)->top = _0; - _4 = cl_0->p; - _8 = _4->sizep; - if (_7 > 0) - goto (OP_RETURN_if_sizep_gt_0_12_0); - else - goto (OP_RETURN_else_sizep_gt_0_12_0); - -OP_TEST_do_skip_5_0: - base_3->value_.b = 1; - base_3->tt_ = 1; - goto (OP_TEST_do_jmp_5_0); + goto (OP_RETURN_else_sizep_gt_0_12_0); OP_RETURN_if_sizep_gt_0_12_0: - luaF_close (L_0(D), base_6); + luaF_close (L_0(D), base_2); OP_RETURN_else_sizep_gt_0_12_0: - _6 = base_6 + 16; - luaD_poscall (L_0(D), _6); + _3 = base_2 + 16; + luaD_poscall (L_0(D), _3); return 1; }