Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin <imun@tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] fold: keep type of emitted CONV in sync with its mode
Date: Thu, 5 Dec 2019 15:51:59 +0300	[thread overview]
Message-ID: <20191205125159.GI1214@tarantool.org> (raw)
In-Reply-To: <20191204145537.GI1718@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 <vegorov@google.com>
> > 
> > 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 <vegorov@google.com>
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> > 
> > 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

  reply	other threads:[~2019-12-05 12:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-02  7:03 Igor Munkin
2019-12-04 14:55 ` Sergey Ostanevich
2019-12-05 12:51   ` Igor Munkin [this message]
2019-12-05 13:09 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191205125159.GI1214@tarantool.org \
    --to=imun@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] fold: keep type of emitted CONV in sync with its mode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox