From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id CDE6F46971A for ; Thu, 5 Dec 2019 15:54:07 +0300 (MSK) Date: Thu, 5 Dec 2019 15:51:59 +0300 From: Igor Munkin Message-ID: <20191205125159.GI1214@tarantool.org> References: <7729408a83dbde42bc7c8a5fa1fd8ba6a7f7c40f.1575269331.git.imun@tarantool.org> <20191204145537.GI1718@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191204145537.GI1718@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] fold: keep type of emitted CONV in sync with its mode List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org Sergos, Thanks for your review and the futher research! Cced Kirill to proceed with the patch. On 04.12.19, Sergey Ostanevich wrote: > Hi! > > Thanks for the patch, I didn't find any similar issues to fix along with > it. > > LGTM, > Sergos > > On 02 Dec 10:03, Igor Munkin wrote: > > From: Vyacheslav Egorov > > > > When emitting CONV make sure that its type matches its destination IRType. > > > > This keeps IR fully internally consistent with respect to types - i.e. if > > we push narrowing CONV Dt.St upwards through an arithmetic operation of type > > St we end up with arithmetic operation of type Dt and two convertions > > CONV Dt.St which narrow the operands. > > > > Igor Munkin: > > * added a test for the problem > > * backported the original patch to tarantool/luajit repo > > * stripped some words not related to the patch itself > > > > Fixes LuaJIT/LuaJIT#524 > > > > Signed-off-by: Vyacheslav Egorov > > Signed-off-by: Igor Munkin > > --- > > > > This is a backport for https://github.com/moonjit/moonjit/commit/4dba12d > > > > Branch: https://github.com/tarantool/luajit/tree/imun/backport-fix-LuaJIT-524 > > Issue: https://github.com/LuaJIT/LuaJIT/issues/524 > > > > src/lj_opt_fold.c | 4 ++-- > > test/fold_bug_LuaJIT_524.test.lua | 24 ++++++++++++++++++++++++ > > 2 files changed, 26 insertions(+), 2 deletions(-) > > create mode 100755 test/fold_bug_LuaJIT_524.test.lua > > > > diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c > > index acbf36a..3c50806 100644 > > --- a/src/lj_opt_fold.c > > +++ b/src/lj_opt_fold.c > > @@ -1261,8 +1261,8 @@ LJFOLDF(simplify_conv_narrow) > > IRType t = irt_type(fins->t); > > IRRef op1 = fleft->op1, op2 = fleft->op2, mode = fins->op2; > > PHIBARRIER(fleft); > > - op1 = emitir(IRTI(IR_CONV), op1, mode); > > - op2 = emitir(IRTI(IR_CONV), op2, mode); > > + op1 = emitir(IRT(IR_CONV, t), op1, mode); > > + op2 = emitir(IRT(IR_CONV, t), op2, mode); > > fins->ot = IRT(op, t); > > fins->op1 = op1; > > fins->op2 = op2; > > diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/fold_bug_LuaJIT_524.test.lua > > new file mode 100755 > > index 0000000..b056684 > > --- /dev/null > > +++ b/test/fold_bug_LuaJIT_524.test.lua > > @@ -0,0 +1,24 @@ > > +#!/usr/bin/env tarantool > > + > > +local tap = require('tap') > > +local ffi = require('ffi') > > + > > +local test = tap.test("LuaJIT 524") > > +test:plan(1) > > + > > +-- Test file to demonstrate LuaJIT folding machinery incorrect behaviour, > > +-- details: > > +-- https://github.com/LuaJIT/LuaJIT/issues/524 > > +-- https://github.com/moonjit/moonjit/issues/37 > > + > > +jit.opt.start(0, "fold", "cse", "fwd", "hotloop=1") > > + > > +local sq = ffi.cast("uint32_t", 42) > > + > > +for _ = 1, 3 do > > + sq = ffi.cast("uint32_t", sq * sq) > > +end > > + > > +test:is(tonumber(sq), math.fmod(math.pow(42, 8), math.pow(2, 32))) > > + > > +test:check() > > -- > > 2.24.0 > > -- Best regards, IM