Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
@ 2021-12-29 12:27 Sergey Kaplun via Tarantool-patches
  2022-01-27 23:18 ` Igor Munkin via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-12-29 12:27 UTC (permalink / raw)
  To: Sergey Ostanevich, Igor Munkin; +Cc: tarantool-patches

From: Mike Pall <mike>

Suggested by spacewander.

(cherry picked from 9ff94c4a1fcec2c310dcb092da694f23186e23)

`maxirconst` should restrict the amount of IR constants for 1 trace.
Nevertheless, its value isn't checked anywhere.

This patch adds the corresponding check after instruction recording.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#6548
---

Issue: https://github.com/LuaJIT/LuaJIT/issues/430
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci

 src/lj_record.c                               |  5 ++-
 .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index 42af09e5..f839ade2 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -2470,8 +2470,9 @@ void lj_record_ins(jit_State *J)
 #undef rbv
 #undef rcv
 
-  /* Limit the number of recorded IR instructions. */
-  if (J->cur.nins > REF_FIRST+(IRRef)J->param[JIT_P_maxrecord])
+  /* Limit the number of recorded IR instructions and constants. */
+  if (J->cur.nins > REF_FIRST+(IRRef)J->param[JIT_P_maxrecord] ||
+      J->cur.nk < REF_BIAS-(IRRef)J->param[JIT_P_maxirconst])
     lj_trace_err(J, LJ_TRERR_TRACEOV);
 }
 
diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
new file mode 100644
index 00000000..1829b37d
--- /dev/null
+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -0,0 +1,43 @@
+-- XXX: avoid any other traces compilation due to hotcount
+-- collisions for predictible results.
+jit.off()
+jit.flush()
+
+-- Disabled on *BSD due to #4819.
+require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
+
+local tap = require('tap')
+
+local test = tap.test('lj-430-maxirconst')
+test:plan(2)
+
+-- XXX: trace always has at least 3 IR constants: for nil, false
+-- and true.
+jit.opt.start('hotloop=1', 'maxirconst=3')
+
+-- This function has only 3 IR constant.
+local function irconst3()
+end
+
+-- This function has 4 IR constants before optimizations.
+local function irconst4()
+  local _ = 42
+end
+
+local ntrace_old = misc.getmetrics().jit_trace_num
+jit.on()
+irconst3()
+irconst3()
+jit.off()
+test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
+	'trace number increases')
+
+ntrace_old = misc.getmetrics().jit_trace_num
+jit.on()
+irconst4()
+irconst4()
+jit.off()
+test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
+	'trace number is the same')
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2021-12-29 12:27 [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit Sergey Kaplun via Tarantool-patches
@ 2022-01-27 23:18 ` Igor Munkin via Tarantool-patches
  2022-01-28  8:42   ` Sergey Kaplun via Tarantool-patches
  2022-01-28 21:45 ` Igor Munkin via Tarantool-patches
  2022-02-11 19:09 ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-01-27 23:18 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for the patch! LGTM, except a couple of nits.

On 29.12.21, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> Suggested by spacewander.
> 
> (cherry picked from 9ff94c4a1fcec2c310dcb092da694f23186e23)

Typo: s/9ff94c4a1fcec2c310dcb092da694f23186e23/0a9ff94c4a1fcec2c310dcb092da694f23186e23/.

> 
> `maxirconst` should restrict the amount of IR constants for 1 trace.

Typo: It's better s/for 1/per/, but feel free to ignore.

> Nevertheless, its value isn't checked anywhere.

Lol, classic.

> 
> This patch adds the corresponding check after instruction recording.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#6548
> ---
> 
> Issue: https://github.com/LuaJIT/LuaJIT/issues/430
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> 
>  src/lj_record.c                               |  5 ++-
>  .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
>  2 files changed, 46 insertions(+), 2 deletions(-)
>  create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
> 

<snipped>

> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> new file mode 100644
> index 00000000..1829b37d
> --- /dev/null
> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
> @@ -0,0 +1,43 @@
> +-- XXX: avoid any other traces compilation due to hotcount
> +-- collisions for predictible results.

Typo: s/predictible/predictable/.

> +jit.off()
> +jit.flush()

Minor: I'd rather move this part closer to 'jit.opt.start' to save the
test structure closer to the other test chunks. Feel free to ignore.

> +
> +-- Disabled on *BSD due to #4819.
> +require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +
> +local tap = require('tap')
> +
> +local test = tap.test('lj-430-maxirconst')
> +test:plan(2)
> +
> +-- XXX: trace always has at least 3 IR constants: for nil, false
> +-- and true.
> +jit.opt.start('hotloop=1', 'maxirconst=3')
> +
> +-- This function has only 3 IR constant.
> +local function irconst3()
> +end
> +
> +-- This function has 4 IR constants before optimizations.
> +local function irconst4()
> +  local _ = 42
> +end
> +
> +local ntrace_old = misc.getmetrics().jit_trace_num
> +jit.on()
> +irconst3()
> +irconst3()
> +jit.off()
> +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
> +	'trace number increases')

Typo: I doubt we use tabs in Lua sources, but I might be wrong...

> +
> +ntrace_old = misc.getmetrics().jit_trace_num
> +jit.on()
> +irconst4()
> +irconst4()
> +jit.off()
> +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
> +	'trace number is the same')

Ditto.

> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.34.1
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-27 23:18 ` Igor Munkin via Tarantool-patches
@ 2022-01-28  8:42   ` Sergey Kaplun via Tarantool-patches
  2022-01-28 11:55     ` Igor Munkin via Tarantool-patches
  0 siblings, 1 reply; 11+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-01-28  8:42 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Igor,

Thanks for the review!

On 28.01.22, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! LGTM, except a couple of nits.
> 
> On 29.12.21, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> > 
> > Suggested by spacewander.
> > 
> > (cherry picked from 9ff94c4a1fcec2c310dcb092da694f23186e23)
> 
> Typo: s/9ff94c4a1fcec2c310dcb092da694f23186e23/0a9ff94c4a1fcec2c310dcb092da694f23186e23/.

Ouch! Fixed.

> 
> > 
> > `maxirconst` should restrict the amount of IR constants for 1 trace.
> 
> Typo: It's better s/for 1/per/, but feel free to ignore.

Fixed.

The new commit message is the following:

===================================================================
Actually implement maxirconst trace limit.

Suggested by spacewander.

(cherry picked from 0a9ff94c4a1fcec2c310dcb092da694f23186e23)

`maxirconst` should restrict the amount of IR constants per trace.
Nevertheless, its value isn't checked anywhere.

This patch adds the corresponding check after instruction recording.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#6548
===================================================================

> 
> > Nevertheless, its value isn't checked anywhere.
> 
> Lol, classic.
> 
> > 
> > This patch adds the corresponding check after instruction recording.
> > 
> > Sergey Kaplun:
> > * added the description and the test for the problem
> > 
> > Part of tarantool/tarantool#6548
> > ---
> > 
> > Issue: https://github.com/LuaJIT/LuaJIT/issues/430
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> > Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> > 
> >  src/lj_record.c                               |  5 ++-
> >  .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
> >  2 files changed, 46 insertions(+), 2 deletions(-)
> >  create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
> > 
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> > new file mode 100644
> > index 00000000..1829b37d
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
> > @@ -0,0 +1,43 @@
> > +-- XXX: avoid any other traces compilation due to hotcount
> > +-- collisions for predictible results.
> 
> Typo: s/predictible/predictable/.

Fixed.

Branch is force-pushed.
===================================================================
diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
index 1829b37d..df17637f 100644
--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -1,5 +1,5 @@
 -- XXX: avoid any other traces compilation due to hotcount
--- collisions for predictible results.
+-- collisions for predictable results.
 jit.off()
 jit.flush()
 
===================================================================

> 
> > +jit.off()
> > +jit.flush()
> 
> Minor: I'd rather move this part closer to 'jit.opt.start' to save the
> test structure closer to the other test chunks. Feel free to ignore.

I really want to exclude __any__ JIT work here to avoid false-positive
hotcount (was precendents during writing this test :)). Ignoring.

> 
> > +
> > +-- Disabled on *BSD due to #4819.
> > +require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> > +
> > +local tap = require('tap')
> > +
> > +local test = tap.test('lj-430-maxirconst')
> > +test:plan(2)
> > +
> > +-- XXX: trace always has at least 3 IR constants: for nil, false
> > +-- and true.
> > +jit.opt.start('hotloop=1', 'maxirconst=3')
> > +
> > +-- This function has only 3 IR constant.
> > +local function irconst3()
> > +end
> > +
> > +-- This function has 4 IR constants before optimizations.
> > +local function irconst4()
> > +  local _ = 42
> > +end
> > +
> > +local ntrace_old = misc.getmetrics().jit_trace_num
> > +jit.on()
> > +irconst3()
> > +irconst3()
> > +jit.off()
> > +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
> > +	'trace number increases')
> 
> Typo: I doubt we use tabs in Lua sources, but I might be wrong...

I suggest to use quarter tabs indent style as we use for C code and Mike
use in src/jit/*.lua (see bcsave.lua for example).

There was no precedent before, IINM :).

> 
> > +
> > +ntrace_old = misc.getmetrics().jit_trace_num
> > +jit.on()
> > +irconst4()
> > +irconst4()
> > +jit.off()
> > +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
> > +	'trace number is the same')
> 
> Ditto.
> 
> > +
> > +os.exit(test:check() and 0 or 1)
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-28  8:42   ` Sergey Kaplun via Tarantool-patches
@ 2022-01-28 11:55     ` Igor Munkin via Tarantool-patches
  2022-01-28 12:35       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-01-28 11:55 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

On 28.01.22, Sergey Kaplun wrote:
> Igor,
> 
> Thanks for the review!
> 

<snipped>

> > > 
> > > Issue: https://github.com/LuaJIT/LuaJIT/issues/430
> > > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> > > Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> > > 
> > >  src/lj_record.c                               |  5 ++-
> > >  .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
> > >  2 files changed, 46 insertions(+), 2 deletions(-)
> > >  create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
> > > 
> > 
> > <snipped>
> > 
> > > diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> > > new file mode 100644
> > > index 00000000..1829b37d
> > > --- /dev/null
> > > +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
> > > @@ -0,0 +1,43 @@
> > > +-- XXX: avoid any other traces compilation due to hotcount
> > > +-- collisions for predictible results.

<snipped>

> > > +jit.off()
> > > +jit.flush()
> > 
> > Minor: I'd rather move this part closer to 'jit.opt.start' to save the
> > test structure closer to the other test chunks. Feel free to ignore.
> 
> I really want to exclude __any__ JIT work here to avoid false-positive
> hotcount (was precendents during writing this test :)). Ignoring.

OK, got it.

> 

<snipped>

> > > +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
> > > +	'trace number increases')
> > 
> > Typo: I doubt we use tabs in Lua sources, but I might be wrong...
> 
> I suggest to use quarter tabs indent style as we use for C code and Mike
> use in src/jit/*.lua (see bcsave.lua for example).
> 
> There was no precedent before, IINM :).

See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].

> 
> > 
> > > +
> > > +ntrace_old = misc.getmetrics().jit_trace_num
> > > +jit.on()
> > > +irconst4()
> > > +irconst4()
> > > +jit.off()
> > > +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
> > > +	'trace number is the same')
> > 
> > Ditto.
> > 
> > > +
> > > +os.exit(test:check() and 0 or 1)
> > > -- 
> > > 2.34.1
> > > 
> > 
> > -- 
> > Best regards,
> > IM
> 
> -- 
> Best regards,
> Sergey Kaplun

[1]: https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-28 11:55     ` Igor Munkin via Tarantool-patches
@ 2022-01-28 12:35       ` Sergey Kaplun via Tarantool-patches
  2022-01-28 14:16         ` sergos via Tarantool-patches
  0 siblings, 1 reply; 11+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-01-28 12:35 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Igor,

On 28.01.22, Igor Munkin wrote:
> Sergey,
> 
> On 28.01.22, Sergey Kaplun wrote:
> > Igor,
> > 
> > Thanks for the review!
> > 
> 
> <snipped>
> 
> > > > 
> > > > Issue: https://github.com/LuaJIT/LuaJIT/issues/430
> > > > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> > > > Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> > > > 
> > > >  src/lj_record.c                               |  5 ++-
> > > >  .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
> > > >  2 files changed, 46 insertions(+), 2 deletions(-)
> > > >  create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
> > > > 
> > > 
> > > <snipped>
> > > 
> > > > diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> > > > new file mode 100644
> > > > index 00000000..1829b37d
> > > > --- /dev/null
> > > > +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
> > > > @@ -0,0 +1,43 @@
> > > > +-- XXX: avoid any other traces compilation due to hotcount
> > > > +-- collisions for predictible results.
> 
> <snipped>
> 
> > > > +jit.off()
> > > > +jit.flush()
> > > 
> > > Minor: I'd rather move this part closer to 'jit.opt.start' to save the
> > > test structure closer to the other test chunks. Feel free to ignore.
> > 
> > I really want to exclude __any__ JIT work here to avoid false-positive
> > hotcount (was precendents during writing this test :)). Ignoring.
> 
> OK, got it.
> 
> > 
> 
> <snipped>
> 
> > > > +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
> > > > +	'trace number increases')
> > > 
> > > Typo: I doubt we use tabs in Lua sources, but I might be wrong...
> > 
> > I suggest to use quarter tabs indent style as we use for C code and Mike
> > use in src/jit/*.lua (see bcsave.lua for example).
> > 
> > There was no precedent before, IINM :).
> 
> See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].

Fixed, repushed.

> 
> > 
> > > 
> > > > +
> > > > +ntrace_old = misc.getmetrics().jit_trace_num
> > > > +jit.on()
> > > > +irconst4()
> > > > +irconst4()
> > > > +jit.off()
> > > > +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
> > > > +	'trace number is the same')
> > > 
> > > Ditto.
> > > 
> > > > +
> > > > +os.exit(test:check() and 0 or 1)
> > > > -- 
> > > > 2.34.1
> > > > 
> > > 
> > > -- 
> > > Best regards,
> > > IM
> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 
> [1]: https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-28 12:35       ` Sergey Kaplun via Tarantool-patches
@ 2022-01-28 14:16         ` sergos via Tarantool-patches
  2022-01-28 15:09           ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 11+ messages in thread
From: sergos via Tarantool-patches @ 2022-01-28 14:16 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 3182 bytes --]

Hi!

I reviewed the one at https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c <https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c>

Overall is LGTM, I would like to update the 2nd test message to something like
’trace should not appear due to maxirconst limit’

Sergos

> On 28 Jan 2022, at 15:35, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Igor,
> 
> On 28.01.22, Igor Munkin wrote:
>> Sergey,
>> 
>> On 28.01.22, Sergey Kaplun wrote:
>>> Igor,
>>> 
>>> Thanks for the review!
>>> 
>> 
>> <snipped>
>> 
>>>>> 
>>>>> Issue: https://github.com/LuaJIT/LuaJIT/issues/430
>>>>> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
>>>>> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
>>>>> 
>>>>> src/lj_record.c                               |  5 ++-
>>>>> .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
>>>>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>>>> create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
>>>>> 
>>>> 
>>>> <snipped>
>>>> 
>>>>> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
>>>>> new file mode 100644
>>>>> index 00000000..1829b37d
>>>>> --- /dev/null
>>>>> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
>>>>> @@ -0,0 +1,43 @@
>>>>> +-- XXX: avoid any other traces compilation due to hotcount
>>>>> +-- collisions for predictible results.
>> 
>> <snipped>
>> 
>>>>> +jit.off()
>>>>> +jit.flush()
>>>> 
>>>> Minor: I'd rather move this part closer to 'jit.opt.start' to save the
>>>> test structure closer to the other test chunks. Feel free to ignore.
>>> 
>>> I really want to exclude __any__ JIT work here to avoid false-positive
>>> hotcount (was precendents during writing this test :)). Ignoring.
>> 
>> OK, got it.
>> 
>>> 
>> 
>> <snipped>
>> 
>>>>> +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
>>>>> +	'trace number increases')
>>>> 
>>>> Typo: I doubt we use tabs in Lua sources, but I might be wrong...
>>> 
>>> I suggest to use quarter tabs indent style as we use for C code and Mike
>>> use in src/jit/*.lua (see bcsave.lua for example).
>>> 
>>> There was no precedent before, IINM :).
>> 
>> See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].
> 
> Fixed, repushed.
> 
>> 
>>> 
>>>> 
>>>>> +
>>>>> +ntrace_old = misc.getmetrics().jit_trace_num
>>>>> +jit.on()
>>>>> +irconst4()
>>>>> +irconst4()
>>>>> +jit.off()
>>>>> +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
>>>>> +	'trace number is the same')
>>>> 
>>>> Ditto.
>>>> 
>>>>> +
>>>>> +os.exit(test:check() and 0 or 1)
>>>>> -- 
>>>>> 2.34.1
>>>>> 
>>>> 
>>>> -- 
>>>> Best regards,
>>>> IM
>>> 
>>> -- 
>>> Best regards,
>>> Sergey Kaplun
>> 
>> [1]: https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua
>> 
>> -- 
>> Best regards,
>> IM
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 12356 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-28 14:16         ` sergos via Tarantool-patches
@ 2022-01-28 15:09           ` Sergey Kaplun via Tarantool-patches
  2022-01-28 21:28             ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 1 reply; 11+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-01-28 15:09 UTC (permalink / raw)
  To: sergos; +Cc: tarantool-patches

Hi, Sergos!

Thanks for the review!

On 28.01.22, sergos wrote:
> Hi!
> 
> I reviewed the one at https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c <https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c>
> 
> Overall is LGTM, I would like to update the 2nd test message to something like
> ’trace should not appear due to maxirconst limit’

Fixed, see the iterative patch below.
Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
index 79553ecb..10de2520 100644
--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -38,6 +38,6 @@ irconst4()
 irconst4()
 jit.off()
 test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
-        'trace number is the same')
+        'trace should not appear due to maxirconst limit')
 
 os.exit(test:check() and 0 or 1)
===================================================================

> 
> Sergos
> 
> > On 28 Jan 2022, at 15:35, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > Igor,
> > 
> > On 28.01.22, Igor Munkin wrote:
> >> Sergey,
> >> 
> >> On 28.01.22, Sergey Kaplun wrote:
> >>> Igor,
> >>> 
> >>> Thanks for the review!
> >>> 
> >> 
> >> <snipped>
> >> 
> >>>>> 
> >>>>> Issue: https://github.com/LuaJIT/LuaJIT/issues/430
> >>>>> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> >>>>> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> >>>>> 
> >>>>> src/lj_record.c                               |  5 ++-
> >>>>> .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
> >>>>> 2 files changed, 46 insertions(+), 2 deletions(-)
> >>>>> create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
> >>>>> 
> >>>> 
> >>>> <snipped>
> >>>> 
> >>>>> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> >>>>> new file mode 100644
> >>>>> index 00000000..1829b37d
> >>>>> --- /dev/null
> >>>>> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
> >>>>> @@ -0,0 +1,43 @@
> >>>>> +-- XXX: avoid any other traces compilation due to hotcount
> >>>>> +-- collisions for predictible results.
> >> 
> >> <snipped>
> >> 
> >>>>> +jit.off()
> >>>>> +jit.flush()
> >>>> 
> >>>> Minor: I'd rather move this part closer to 'jit.opt.start' to save the
> >>>> test structure closer to the other test chunks. Feel free to ignore.
> >>> 
> >>> I really want to exclude __any__ JIT work here to avoid false-positive
> >>> hotcount (was precendents during writing this test :)). Ignoring.
> >> 
> >> OK, got it.
> >> 
> >>> 
> >> 
> >> <snipped>
> >> 
> >>>>> +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
> >>>>> +	'trace number increases')
> >>>> 
> >>>> Typo: I doubt we use tabs in Lua sources, but I might be wrong...
> >>> 
> >>> I suggest to use quarter tabs indent style as we use for C code and Mike
> >>> use in src/jit/*.lua (see bcsave.lua for example).
> >>> 
> >>> There was no precedent before, IINM :).
> >> 
> >> See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].
> > 
> > Fixed, repushed.
> > 
> >> 
> >>> 
> >>>> 
> >>>>> +
> >>>>> +ntrace_old = misc.getmetrics().jit_trace_num
> >>>>> +jit.on()
> >>>>> +irconst4()
> >>>>> +irconst4()
> >>>>> +jit.off()
> >>>>> +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
> >>>>> +	'trace number is the same')
> >>>> 
> >>>> Ditto.
> >>>> 
> >>>>> +
> >>>>> +os.exit(test:check() and 0 or 1)
> >>>>> -- 
> >>>>> 2.34.1
> >>>>> 
> >>>> 
> >>>> -- 
> >>>> Best regards,
> >>>> IM
> >>> 
> >>> -- 
> >>> Best regards,
> >>> Sergey Kaplun
> >> 
> >> [1]: https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua
> >> 
> >> -- 
> >> Best regards,
> >> IM
> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches]  [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-28 15:09           ` Sergey Kaplun via Tarantool-patches
@ 2022-01-28 21:28             ` Sergey Ostanevich via Tarantool-patches
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Ostanevich via Tarantool-patches @ 2022-01-28 21:28 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 4383 bytes --]


Just to avoid misreading : LGTM

Best regards,
Sergos 

Friday, 28 January 2022, 18:11 +0300 from Kaplun Sergey  <skaplun@tarantool.org>:
>Hi, Sergos!
>
>Thanks for the review!
>
>On 28.01.22, sergos wrote:
>> Hi!
>> 
>> I reviewed the one at  https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c < https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6b068cf12c >
>> 
>> Overall is LGTM, I would like to update the 2nd test message to something like
>> ’trace should not appear due to maxirconst limit’
>
>Fixed, see the iterative patch below.
>Branch is force-pushed.
>
>===================================================================
>diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
>index 79553ecb..10de2520 100644
>--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
>+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
>@@ -38,6 +38,6 @@ irconst4()
> irconst4()
> jit.off()
> test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
>-        'trace number is the same')
>+        'trace should not appear due to maxirconst limit')
> 
> os.exit(test:check() and 0 or 1)
>===================================================================
>
>> 
>> Sergos
>> 
>> > On 28 Jan 2022, at 15:35, Sergey Kaplun < skaplun@tarantool.org > wrote:
>> > 
>> > Igor,
>> > 
>> > On 28.01.22, Igor Munkin wrote:
>> >> Sergey,
>> >> 
>> >> On 28.01.22, Sergey Kaplun wrote:
>> >>> Igor,
>> >>> 
>> >>> Thanks for the review!
>> >>> 
>> >> 
>> >> <snipped>
>> >> 
>> >>>>> 
>> >>>>> Issue:  https://github.com/LuaJIT/LuaJIT/issues/430
>> >>>>> Branch:  https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
>> >>>>> Tarantool branch:  https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
>> >>>>> 
>> >>>>> src/lj_record.c                               |  5 ++-
>> >>>>> .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
>> >>>>> 2 files changed, 46 insertions(+), 2 deletions(-)
>> >>>>> create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
>> >>>>> 
>> >>>> 
>> >>>> <snipped>
>> >>>> 
>> >>>>> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
>> >>>>> new file mode 100644
>> >>>>> index 00000000..1829b37d
>> >>>>> --- /dev/null
>> >>>>> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
>> >>>>> @@ -0,0 +1,43 @@
>> >>>>> +-- XXX: avoid any other traces compilation due to hotcount
>> >>>>> +-- collisions for predictible results.
>> >> 
>> >> <snipped>
>> >> 
>> >>>>> +jit.off()
>> >>>>> +jit.flush()
>> >>>> 
>> >>>> Minor: I'd rather move this part closer to 'jit.opt.start' to save the
>> >>>> test structure closer to the other test chunks. Feel free to ignore.
>> >>> 
>> >>> I really want to exclude __any__ JIT work here to avoid false-positive
>> >>> hotcount (was precendents during writing this test :)). Ignoring.
>> >> 
>> >> OK, got it.
>> >> 
>> >>> 
>> >> 
>> >> <snipped>
>> >> 
>> >>>>> +test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
>> >>>>> +	'trace number increases')
>> >>>> 
>> >>>> Typo: I doubt we use tabs in Lua sources, but I might be wrong...
>> >>> 
>> >>> I suggest to use quarter tabs indent style as we use for C code and Mike
>> >>> use in src/jit/*.lua (see bcsave.lua for example).
>> >>> 
>> >>> There was no precedent before, IINM :).
>> >> 
>> >> See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].
>> > 
>> > Fixed, repushed.
>> > 
>> >> 
>> >>> 
>> >>>> 
>> >>>>> +
>> >>>>> +ntrace_old = misc.getmetrics().jit_trace_num
>> >>>>> +jit.on()
>> >>>>> +irconst4()
>> >>>>> +irconst4()
>> >>>>> +jit.off()
>> >>>>> +test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
>> >>>>> +	'trace number is the same')
>> >>>> 
>> >>>> Ditto.
>> >>>> 
>> >>>>> +
>> >>>>> +os.exit(test:check() and 0 or 1)
>> >>>>> -- 
>> >>>>> 2.34.1
>> >>>>> 
>> >>>> 
>> >>>> -- 
>> >>>> Best regards,
>> >>>> IM
>> >>> 
>> >>> -- 
>> >>> Best regards,
>> >>> Sergey Kaplun
>> >> 
>> >> [1]:  https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj-695-ffi-vararg-call.test.lua
>> >> 
>> >> -- 
>> >> Best regards,
>> >> IM
>> > 
>> > -- 
>> > Best regards,
>> > Sergey Kaplun
>> 
>
>-- 
>Best regards,
>Sergey Kaplun

[-- Attachment #2: Type: text/html, Size: 9324 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2021-12-29 12:27 [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit Sergey Kaplun via Tarantool-patches
  2022-01-27 23:18 ` Igor Munkin via Tarantool-patches
@ 2022-01-28 21:45 ` Igor Munkin via Tarantool-patches
  2022-01-29  1:17   ` Sergey Kaplun via Tarantool-patches
  2022-02-11 19:09 ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-01-28 21:45 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Bad things happen... I was much concerned about using <misc> module in
the test, but I failed to understand why exactly. Now I finally got it
(the hard way): <misc> is missing in 1.10 which is one of the target
branches for this patch. Could you please adjust the test using
<jit.util.traceinfo> helper or via any other way to check that only the
first trace is compiled?

P.S. many thanks to luacheck:
| $ make -j test
| Consolidate compiler generated dependencies of target minilua
| Consolidate compiler generated dependencies of target libmixcframe
| Consolidate compiler generated dependencies of target libsandwich
| Consolidate compiler generated dependencies of target lib1
| Consolidate compiler generated dependencies of target lib2
| Consolidate compiler generated dependencies of target lib21
| Consolidate compiler generated dependencies of target lib11
| Consolidate compiler generated dependencies of target libflush
| [  3%] Built target libsandwich
| [  3%] Built target libmixcframe
| Running luacheck static analysis
| [  8%] Built target lib1
| [ 11%] Built target lib21
| [  6%] Built target minilua
| [  6%] Built target lib2
| [ 13%] Built target lib11
| [ 15%] Built target libflush
| [ 16%] Generating buildvm_arch.h
| Create directory for PUC-Rio Lua 5.1 tests
| [ 16%] Built target PUC-Rio-Lua-5.1-tests-prepare
| Checking /home/imun/projects/tarantool-luajit-maintain/test/luajit-test-init.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-4773-tonumber-fail-on-NUL-char.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-6227-bytecode-allocator-for-comparisons.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-6371-string-char-no-arg.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-375-ir-bufput-signed-char.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua 4 warnings
| 
|     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:27:20: (W113) accessing undefined variable misc
|     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:32:27: (W113) accessing undefined variable misc
|     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:35:14: (W113) accessing undefined variable misc
|     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:40:23: (W113) accessing undefined variable misc
| 
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-601-fix-gc-finderrfunc.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-flush-on-trace.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/or-232-unsink-64-kptr.test.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/tap.lua OK
| Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/utils.lua OK
| 
| Total: 4 warnings / 0 errors in 18 files
| make[3]: *** [test/CMakeFiles/LuaJIT-luacheck.dir/build.make:331: LuaJIT-luacheck] Error 1
| make[2]: *** [CMakeFiles/Makefile2:731: test/CMakeFiles/LuaJIT-luacheck.dir/all] Error 2
| make[2]: *** Waiting for unfinished jobs....
| [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_fold.c.o
| [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_lib.c.o
| [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_asm.c.o
| [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm.c.o
| [ 22%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_peobj.c.o
| [ 22%] Linking C executable buildvm
| [ 22%] Built target buildvm
| make[1]: *** [CMakeFiles/Makefile2:683: test/CMakeFiles/test.dir/rule] Error 2
| make: *** [Makefile:351: test] Error 2

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2022-01-28 21:45 ` Igor Munkin via Tarantool-patches
@ 2022-01-29  1:17   ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-01-29  1:17 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Igor,

On 29.01.22, Igor Munkin wrote:
> Sergey,
> 
> Bad things happen... I was much concerned about using <misc> module in
> the test, but I failed to understand why exactly. Now I finally got it
> (the hard way): <misc> is missing in 1.10 which is one of the target
> branches for this patch. Could you please adjust the test using
> <jit.util.traceinfo> helper or via any other way to check that only the
> first trace is compiled?

Fixed, see the iterative patch below:
Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
index 10de2520..45f0bb1d 100644
--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -7,6 +7,7 @@ jit.flush()
 require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
 
 local tap = require('tap')
+local traceinfo = require('jit.util').traceinfo
 
 local test = tap.test('lj-430-maxirconst')
 test:plan(2)
@@ -24,20 +25,17 @@ local function irconst4()
   local _ = 42
 end
 
-local ntrace_old = misc.getmetrics().jit_trace_num
+assert(not traceinfo(1), 'no any traces')
 jit.on()
 irconst3()
 irconst3()
 jit.off()
-test:ok(ntrace_old + 1 == misc.getmetrics().jit_trace_num,
-        'trace number increases')
+test:ok(traceinfo(1), 'new trace created')
 
-ntrace_old = misc.getmetrics().jit_trace_num
 jit.on()
 irconst4()
 irconst4()
 jit.off()
-test:ok(ntrace_old == misc.getmetrics().jit_trace_num,
-        'trace should not appear due to maxirconst limit')
+test:ok(not traceinfo(2), 'trace should not appear due to maxirconst limit')
 
 os.exit(test:check() and 0 or 1)
===================================================================

> 
> P.S. many thanks to luacheck:
> | $ make -j test
> | Consolidate compiler generated dependencies of target minilua
> | Consolidate compiler generated dependencies of target libmixcframe
> | Consolidate compiler generated dependencies of target libsandwich
> | Consolidate compiler generated dependencies of target lib1
> | Consolidate compiler generated dependencies of target lib2
> | Consolidate compiler generated dependencies of target lib21
> | Consolidate compiler generated dependencies of target lib11
> | Consolidate compiler generated dependencies of target libflush
> | [  3%] Built target libsandwich
> | [  3%] Built target libmixcframe
> | Running luacheck static analysis
> | [  8%] Built target lib1
> | [ 11%] Built target lib21
> | [  6%] Built target minilua
> | [  6%] Built target lib2
> | [ 13%] Built target lib11
> | [ 15%] Built target libflush
> | [ 16%] Generating buildvm_arch.h
> | Create directory for PUC-Rio Lua 5.1 tests
> | [ 16%] Built target PUC-Rio-Lua-5.1-tests-prepare
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/luajit-test-init.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-4773-tonumber-fail-on-NUL-char.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-6227-bytecode-allocator-for-comparisons.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/gh-6371-string-char-no-arg.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-375-ir-bufput-signed-char.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua 4 warnings
> | 
> |     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:27:20: (W113) accessing undefined variable misc
> |     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:32:27: (W113) accessing undefined variable misc
> |     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:35:14: (W113) accessing undefined variable misc
> |     /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-430-maxirconst.test.lua:40:23: (W113) accessing undefined variable misc
> | 
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-601-fix-gc-finderrfunc.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/lj-flush-on-trace.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/or-232-unsink-64-kptr.test.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/tap.lua OK
> | Checking /home/imun/projects/tarantool-luajit-maintain/test/tarantool-tests/utils.lua OK
> | 
> | Total: 4 warnings / 0 errors in 18 files
> | make[3]: *** [test/CMakeFiles/LuaJIT-luacheck.dir/build.make:331: LuaJIT-luacheck] Error 1
> | make[2]: *** [CMakeFiles/Makefile2:731: test/CMakeFiles/LuaJIT-luacheck.dir/all] Error 2
> | make[2]: *** Waiting for unfinished jobs....
> | [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_fold.c.o
> | [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_lib.c.o
> | [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_asm.c.o
> | [ 20%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm.c.o
> | [ 22%] Building C object src/host/CMakeFiles/buildvm.dir/buildvm_peobj.c.o
> | [ 22%] Linking C executable buildvm
> | [ 22%] Built target buildvm
> | make[1]: *** [CMakeFiles/Makefile2:683: test/CMakeFiles/test.dir/rule] Error 2
> | make: *** [Makefile:351: test] Error 2
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit.
  2021-12-29 12:27 [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit Sergey Kaplun via Tarantool-patches
  2022-01-27 23:18 ` Igor Munkin via Tarantool-patches
  2022-01-28 21:45 ` Igor Munkin via Tarantool-patches
@ 2022-02-11 19:09 ` Igor Munkin via Tarantool-patches
  2 siblings, 0 replies; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-02-11 19:09 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

I've checked the patch into all long-term branches in tarantool/luajit
and bumped a new version in 1.10, 2.8 and master.

On 29.12.21, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> Suggested by spacewander.
> 
> (cherry picked from 9ff94c4a1fcec2c310dcb092da694f23186e23)
> 
> `maxirconst` should restrict the amount of IR constants for 1 trace.
> Nevertheless, its value isn't checked anywhere.
> 
> This patch adds the corresponding check after instruction recording.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#6548
> ---
> 
> Issue: https://github.com/LuaJIT/LuaJIT/issues/430
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actually-implement-full-ci
> 
>  src/lj_record.c                               |  5 ++-
>  .../lj-430-maxirconst.test.lua                | 43 +++++++++++++++++++
>  2 files changed, 46 insertions(+), 2 deletions(-)
>  create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua
> 

<snipped>

> -- 
> 2.34.1
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-02-11 19:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-29 12:27 [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit Sergey Kaplun via Tarantool-patches
2022-01-27 23:18 ` Igor Munkin via Tarantool-patches
2022-01-28  8:42   ` Sergey Kaplun via Tarantool-patches
2022-01-28 11:55     ` Igor Munkin via Tarantool-patches
2022-01-28 12:35       ` Sergey Kaplun via Tarantool-patches
2022-01-28 14:16         ` sergos via Tarantool-patches
2022-01-28 15:09           ` Sergey Kaplun via Tarantool-patches
2022-01-28 21:28             ` Sergey Ostanevich via Tarantool-patches
2022-01-28 21:45 ` Igor Munkin via Tarantool-patches
2022-01-29  1:17   ` Sergey Kaplun via Tarantool-patches
2022-02-11 19:09 ` Igor Munkin via Tarantool-patches

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