Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
@ 2021-10-18 18:53 Sergey Kaplun via Tarantool-patches
  2021-10-27 16:08 ` sergos via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-10-18 18:53 UTC (permalink / raw)
  To: Sergey Ostanevich, Igor Munkin; +Cc: tarantool-patches

From: Mike Pall <mike>

Reported by Matthew Burk.

(cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)

The simplify_conv_sext optimization is used for reduction of widening.
cdata indexing narrow optimization uses it for narrowing of a C array
index. The optimization eliminates sign extension for corresponding
integer value. However, this conversion cannot be omitted for non
constant values (for example loading stack slots) as far as their sign
extension may change. The emitted machine code may be incorrect without
aforementioned conversion (for example mov instruction instead movsxd is
used on x86 architecture). As a result the value in a destination
register during trace execution is invalid.

This patch allows this optimization only for constant integer values.

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

Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext

 src/lj_opt_fold.c                             |  2 +-
 .../lj-fix-fold-simplify-conv-sext.test.lua   | 35 +++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua

diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c
index 3c508062..276dc040 100644
--- a/src/lj_opt_fold.c
+++ b/src/lj_opt_fold.c
@@ -1227,7 +1227,7 @@ LJFOLDF(simplify_conv_sext)
   if (ref == J->scev.idx) {
     IRRef lo = J->scev.dir ? J->scev.start : J->scev.stop;
     lua_assert(irt_isint(J->scev.t));
-    if (lo && IR(lo)->i + ofs >= 0) {
+    if (lo && IR(lo)->o == IR_KINT && IR(lo)->i + ofs >= 0) {
     ok_reduce:
 #if LJ_TARGET_X64
       /* Eliminate widening. All 32 bit ops do an implicit zero-extension. */
diff --git a/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
new file mode 100644
index 00000000..bd3738c5
--- /dev/null
+++ b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
@@ -0,0 +1,35 @@
+local tap = require('tap')
+local ffi = require('ffi')
+
+local test = tap.test('lj-fix-fold-simplify-conv-sext')
+
+local NSAMPLES = 4
+local NTEST = NSAMPLES * 2 + 1
+test:plan(NTEST)
+
+local samples = ffi.new('int [?]', NSAMPLES)
+
+-- Prepare data.
+for i = 0, NSAMPLES - 1 do samples[i] = i end
+
+local expected = {3, 2, 1, 0, 3, 2, 1}
+
+local START = 3
+local STOP = -START
+
+local results = {}
+jit.opt.start('hotloop=1')
+for i = START, STOP, -1 do
+  -- While recording cdata indexing the fold CONV SEXT
+  -- optimization eliminate sign extension for the corresponding
+  -- non constant value (i.e. stack slot). As a result the read
+  -- out of bounds was occurring.
+  results[#results + 1] = samples[i % NSAMPLES]
+end
+
+for i = 1, NTEST do
+  test:ok(results[i] == expected[i], 'correct cdata indexing')
+end
+
+os.exit(test:check() and 0 or 1)
+
-- 
2.31.0


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

* Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
  2021-10-18 18:53 [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening Sergey Kaplun via Tarantool-patches
@ 2021-10-27 16:08 ` sergos via Tarantool-patches
  2021-10-28 11:36   ` Sergey Kaplun via Tarantool-patches
  2022-06-28 13:21 ` Igor Munkin via Tarantool-patches
  2022-06-30 12:09 ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 7+ messages in thread
From: sergos via Tarantool-patches @ 2021-10-27 16:08 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Hi!

Thanks for the patch, see my comments below.

Regards,
Sergos

> On 18 Oct 2021, at 21:53, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> Reported by Matthew Burk.
> 
> (cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)
> 
> The simplify_conv_sext optimization is used for reduction of widening.

Whether it is part of narrowing itself? 

> cdata indexing narrow optimization uses it for narrowing of a C array

The sentence is started with lowercase, making one to track back the start
of the sentence. “A narrow optimization for cdata…” in conjunction of
first sentence above should help.


> index. The optimization eliminates sign extension for corresponding
						     the

> integer value. However, this conversion cannot be omitted for non
> constant values (for example loading stack slots) as far as their sign
> extension may change. The emitted machine code may be incorrect without

I believe you meant ‘with’ the conversion. 

> aforementioned conversion (for example mov instruction instead movsxd is
> used on x86 architecture). As a result the value in a destination

The example is too much. Just “negative offset from the stack pointer
may appear positive and result in undefined memory access”
 
> register during trace execution is invalid.
> 
> This patch allows this optimization only for constant integer values.

Should it check if integer - even a constant one - is positive? 

> 
> Sergey Kaplun:
> * added the description and the test for the problem
> ---
> 
> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
> 
> src/lj_opt_fold.c                             |  2 +-
> .../lj-fix-fold-simplify-conv-sext.test.lua   | 35 +++++++++++++++++++
> 2 files changed, 36 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> 
> diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c
> index 3c508062..276dc040 100644
> --- a/src/lj_opt_fold.c
> +++ b/src/lj_opt_fold.c
> @@ -1227,7 +1227,7 @@ LJFOLDF(simplify_conv_sext)
>   if (ref == J->scev.idx) {
>     IRRef lo = J->scev.dir ? J->scev.start : J->scev.stop;
>     lua_assert(irt_isint(J->scev.t));
> -    if (lo && IR(lo)->i + ofs >= 0) {
> +    if (lo && IR(lo)->o == IR_KINT && IR(lo)->i + ofs >= 0) {
>     ok_reduce:
> #if LJ_TARGET_X64
>       /* Eliminate widening. All 32 bit ops do an implicit zero-extension. */
> diff --git a/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> new file mode 100644
> index 00000000..bd3738c5
> --- /dev/null
> +++ b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> @@ -0,0 +1,35 @@
> +local tap = require('tap')
> +local ffi = require('ffi')
> +
> +local test = tap.test('lj-fix-fold-simplify-conv-sext')
> +
> +local NSAMPLES = 4
> +local NTEST = NSAMPLES * 2 + 1
> +test:plan(NTEST)
> +
> +local samples = ffi.new('int [?]', NSAMPLES)
> +
> +-- Prepare data.
> +for i = 0, NSAMPLES - 1 do samples[i] = i end
> +
> +local expected = {3, 2, 1, 0, 3, 2, 1}
> +
> +local START = 3
> +local STOP = -START
> +
> +local results = {}
> +jit.opt.start('hotloop=1')
> +for i = START, STOP, -1 do
> +  -- While recording cdata indexing the fold CONV SEXT
> +  -- optimization eliminate sign extension for the corresponding
> +  -- non constant value (i.e. stack slot). As a result the read
> +  -- out of bounds was occurring.
> +  results[#results + 1] = samples[i % NSAMPLES]
> +end
> +
> +for i = 1, NTEST do
> +  test:ok(results[i] == expected[i], 'correct cdata indexing')
> +end
> +
> +os.exit(test:check() and 0 or 1)
> +
> -- 
> 2.31.0
> 


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

* Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
  2021-10-27 16:08 ` sergos via Tarantool-patches
@ 2021-10-28 11:36   ` Sergey Kaplun via Tarantool-patches
  2021-10-28 12:37     ` sergos via Tarantool-patches
  0 siblings, 1 reply; 7+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-10-28 11:36 UTC (permalink / raw)
  To: sergos; +Cc: tarantool-patches

Hi, Sergos!

Thanks for the review!

On 27.10.21, sergos wrote:
> Hi!
> 
> Thanks for the patch, see my comments below.
> 
> Regards,
> Sergos
> 
> > On 18 Oct 2021, at 21:53, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > From: Mike Pall <mike>
> > 
> > Reported by Matthew Burk.
> > 
> > (cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)
> > 
> > The simplify_conv_sext optimization is used for reduction of widening.
> 
> Whether it is part of narrowing itself? 

Not exactly, it is fold optimization that can be used during narrowing.

lj_opt_narrow_cindex -> lj_opt_fold (in emitir) -> fold_narrow_convert ->
lj_opt_narrow_convert -> narrow_conv_emit -> lj_opt_fold ->
fold_simplify_conv_sext

> 
> > cdata indexing narrow optimization uses it for narrowing of a C array
> 
> The sentence is started with lowercase, making one to track back the start
> of the sentence. “A narrow optimization for cdata…” in conjunction of
> first sentence above should help.
> 
> 
> > index. The optimization eliminates sign extension for corresponding
> 						     the
> 
> > integer value. However, this conversion cannot be omitted for non
> > constant values (for example loading stack slots) as far as their sign
> > extension may change. The emitted machine code may be incorrect without
> 
> I believe you meant ‘with’ the conversion. 
> 
> > aforementioned conversion (for example mov instruction instead movsxd is
> > used on x86 architecture). As a result the value in a destination
> 
> The example is too much. Just “negative offset from the stack pointer
> may appear positive and result in undefined memory access”

Fixed all your comments, see the updated commit message below.
Branch is force pushed.
===================================================================
Fix FOLD rule for strength reduction of widening.

Reported by Matthew Burk.

(cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)

The simplify_conv_sext optimization is used for reduction of widening.
An indexing narrow optimization for cdata uses it for narrowing of a C
array index. The optimization eliminates sign extension for the
corresponding integer value. However, this conversion cannot be omitted
for non constant values (for example loading stack slots) as far as
their sign extension may change. The emitted machine code may be
incorrect with aforementioned conversion (negative offset from the stack
pointer may appear positive and result is undefined memory access). As a
result the value in a destination register during trace execution is
invalid.

This patch allows this optimization only for constant integer values.

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

===================================================================
>  
> > register during trace execution is invalid.
> > 
> > This patch allows this optimization only for constant integer values.
> 
> Should it check if integer - even a constant one - is positive? 

No, why? In fold rule are declared both types: INT64 and UINT64.

> 
> > 
> > Sergey Kaplun:
> > * added the description and the test for the problem
> > ---
> > 
> > Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
> > 
> > src/lj_opt_fold.c                             |  2 +-
> > .../lj-fix-fold-simplify-conv-sext.test.lua   | 35 +++++++++++++++++++
> > 2 files changed, 36 insertions(+), 1 deletion(-)
> > create mode 100644 test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> > 
> > diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c
> > index 3c508062..276dc040 100644
> > --- a/src/lj_opt_fold.c
> > +++ b/src/lj_opt_fold.c
> > @@ -1227,7 +1227,7 @@ LJFOLDF(simplify_conv_sext)
> >   if (ref == J->scev.idx) {
> >     IRRef lo = J->scev.dir ? J->scev.start : J->scev.stop;
> >     lua_assert(irt_isint(J->scev.t));
> > -    if (lo && IR(lo)->i + ofs >= 0) {
> > +    if (lo && IR(lo)->o == IR_KINT && IR(lo)->i + ofs >= 0) {
> >     ok_reduce:
> > #if LJ_TARGET_X64
> >       /* Eliminate widening. All 32 bit ops do an implicit zero-extension. */
> > diff --git a/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> > new file mode 100644
> > index 00000000..bd3738c5
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> > @@ -0,0 +1,35 @@
> > +local tap = require('tap')
> > +local ffi = require('ffi')
> > +
> > +local test = tap.test('lj-fix-fold-simplify-conv-sext')
> > +
> > +local NSAMPLES = 4
> > +local NTEST = NSAMPLES * 2 + 1

Also fixed typo here: s/+/-/

> > +test:plan(NTEST)
> > +
> > +local samples = ffi.new('int [?]', NSAMPLES)
> > +
> > +-- Prepare data.
> > +for i = 0, NSAMPLES - 1 do samples[i] = i end
> > +
> > +local expected = {3, 2, 1, 0, 3, 2, 1}
> > +
> > +local START = 3
> > +local STOP = -START
> > +
> > +local results = {}
> > +jit.opt.start('hotloop=1')
> > +for i = START, STOP, -1 do
> > +  -- While recording cdata indexing the fold CONV SEXT
> > +  -- optimization eliminate sign extension for the corresponding
> > +  -- non constant value (i.e. stack slot). As a result the read
> > +  -- out of bounds was occurring.
> > +  results[#results + 1] = samples[i % NSAMPLES]
> > +end
> > +
> > +for i = 1, NTEST do
> > +  test:ok(results[i] == expected[i], 'correct cdata indexing')
> > +end
> > +
> > +os.exit(test:check() and 0 or 1)
> > +
> > -- 
> > 2.31.0
> > 
> 

-- 
Best regards,
Sergey Kaplun

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

* Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
  2021-10-28 11:36   ` Sergey Kaplun via Tarantool-patches
@ 2021-10-28 12:37     ` sergos via Tarantool-patches
  2021-10-29  7:49       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 7+ messages in thread
From: sergos via Tarantool-patches @ 2021-10-28 12:37 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

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

Hi! 

The description looks good!

Although, I can’t get the point how this

> (negative offset from the stack
> pointer may appear positive and result is undefined memory access). 

can’t be applied to a constant? Hence, we’d need a check?

Although, I’m trying to get into Mike’s business again. So it LGTM as a backport.

Regards,
Sergos


> On 28 Oct 2021, at 14:36, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Hi, Sergos!
> 
> Thanks for the review!
> 
> On 27.10.21, sergos wrote:
>> Hi!
>> 
>> Thanks for the patch, see my comments below.
>> 
>> Regards,
>> Sergos
>> 
>>> On 18 Oct 2021, at 21:53, Sergey Kaplun <skaplun@tarantool.org> wrote:
>>> 
>>> From: Mike Pall <mike>
>>> 
>>> Reported by Matthew Burk.
>>> 
>>> (cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)
>>> 
>>> The simplify_conv_sext optimization is used for reduction of widening.
>> 
>> Whether it is part of narrowing itself? 
> 
> Not exactly, it is fold optimization that can be used during narrowing.
> 
> lj_opt_narrow_cindex -> lj_opt_fold (in emitir) -> fold_narrow_convert ->
> lj_opt_narrow_convert -> narrow_conv_emit -> lj_opt_fold ->
> fold_simplify_conv_sext
> 
>> 
>>> cdata indexing narrow optimization uses it for narrowing of a C array
>> 
>> The sentence is started with lowercase, making one to track back the start
>> of the sentence. “A narrow optimization for cdata…” in conjunction of
>> first sentence above should help.
>> 
>> 
>>> index. The optimization eliminates sign extension for corresponding
>> 						     the
>> 
>>> integer value. However, this conversion cannot be omitted for non
>>> constant values (for example loading stack slots) as far as their sign
>>> extension may change. The emitted machine code may be incorrect without
>> 
>> I believe you meant ‘with’ the conversion. 
>> 
>>> aforementioned conversion (for example mov instruction instead movsxd is
>>> used on x86 architecture). As a result the value in a destination
>> 
>> The example is too much. Just “negative offset from the stack pointer
>> may appear positive and result in undefined memory access”
> 
> Fixed all your comments, see the updated commit message below.
> Branch is force pushed.
> ===================================================================
> Fix FOLD rule for strength reduction of widening.
> 
> Reported by Matthew Burk.
> 
> (cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)
> 
> The simplify_conv_sext optimization is used for reduction of widening.
> An indexing narrow optimization for cdata uses it for narrowing of a C
> array index. The optimization eliminates sign extension for the
> corresponding integer value. However, this conversion cannot be omitted
> for non constant values (for example loading stack slots) as far as
> their sign extension may change. The emitted machine code may be
> incorrect with aforementioned conversion (negative offset from the stack
> pointer may appear positive and result is undefined memory access). As a
> result the value in a destination register during trace execution is
> invalid.
> 
> This patch allows this optimization only for constant integer values.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> ===================================================================
>> 
>>> register during trace execution is invalid.
>>> 
>>> This patch allows this optimization only for constant integer values.
>> 
>> Should it check if integer - even a constant one - is positive? 
> 
> No, why? In fold rule are declared both types: INT64 and UINT64.
> 
>> 
>>> 
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>>> ---
>>> 
>>> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
>>> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
>>> 
>>> src/lj_opt_fold.c                             |  2 +-
>>> .../lj-fix-fold-simplify-conv-sext.test.lua   | 35 +++++++++++++++++++
>>> 2 files changed, 36 insertions(+), 1 deletion(-)
>>> create mode 100644 test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
>>> 
>>> diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c
>>> index 3c508062..276dc040 100644
>>> --- a/src/lj_opt_fold.c
>>> +++ b/src/lj_opt_fold.c
>>> @@ -1227,7 +1227,7 @@ LJFOLDF(simplify_conv_sext)
>>>  if (ref == J->scev.idx) {
>>>    IRRef lo = J->scev.dir ? J->scev.start : J->scev.stop;
>>>    lua_assert(irt_isint(J->scev.t));
>>> -    if (lo && IR(lo)->i + ofs >= 0) {
>>> +    if (lo && IR(lo)->o == IR_KINT && IR(lo)->i + ofs >= 0) {
>>>    ok_reduce:
>>> #if LJ_TARGET_X64
>>>      /* Eliminate widening. All 32 bit ops do an implicit zero-extension. */
>>> diff --git a/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
>>> new file mode 100644
>>> index 00000000..bd3738c5
>>> --- /dev/null
>>> +++ b/test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
>>> @@ -0,0 +1,35 @@
>>> +local tap = require('tap')
>>> +local ffi = require('ffi')
>>> +
>>> +local test = tap.test('lj-fix-fold-simplify-conv-sext')
>>> +
>>> +local NSAMPLES = 4
>>> +local NTEST = NSAMPLES * 2 + 1
> 
> Also fixed typo here: s/+/-/
> 
>>> +test:plan(NTEST)
>>> +
>>> +local samples = ffi.new('int [?]', NSAMPLES)
>>> +
>>> +-- Prepare data.
>>> +for i = 0, NSAMPLES - 1 do samples[i] = i end
>>> +
>>> +local expected = {3, 2, 1, 0, 3, 2, 1}
>>> +
>>> +local START = 3
>>> +local STOP = -START
>>> +
>>> +local results = {}
>>> +jit.opt.start('hotloop=1')
>>> +for i = START, STOP, -1 do
>>> +  -- While recording cdata indexing the fold CONV SEXT
>>> +  -- optimization eliminate sign extension for the corresponding
>>> +  -- non constant value (i.e. stack slot). As a result the read
>>> +  -- out of bounds was occurring.
>>> +  results[#results + 1] = samples[i % NSAMPLES]
>>> +end
>>> +
>>> +for i = 1, NTEST do
>>> +  test:ok(results[i] == expected[i], 'correct cdata indexing')
>>> +end
>>> +
>>> +os.exit(test:check() and 0 or 1)
>>> +
>>> -- 
>>> 2.31.0
>>> 
>> 
> 
> -- 
> Best regards,
> Sergey Kaplun


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

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

* Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
  2021-10-28 12:37     ` sergos via Tarantool-patches
@ 2021-10-29  7:49       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 7+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-10-29  7:49 UTC (permalink / raw)
  To: sergos; +Cc: tarantool-patches

Hi!

On 28.10.21, sergos wrote:
> Hi! 
> 
> The description looks good!
> 
> Although, I can’t get the point how this
> 
> > (negative offset from the stack
> > pointer may appear positive and result is undefined memory access). 
> 
> can’t be applied to a constant? Hence, we’d need a check?
> 
> >> Should it check if integer - even a constant one - is positive? 
> > 
> > No, why? In fold rule are declared both types: INT64 and UINT64.

Sorry, misinterpreted your question here.
Yes, there is the check
| lo && IR(lo)->o == IR_KINT && IR(lo)->i + ofs >= 0
that the result (constant) integer value for this IR is not negative and we don't
lose its sign and can use the cheaper instruction.

> Although, I’m trying to get into Mike’s business again. So it LGTM as a backport.
> 
> Regards,
> Sergos
> 
> 

<snipped>

> > -- 
> > Best regards,
> > Sergey Kaplun
> 

-- 
Best regards,
Sergey Kaplun

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

* Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
  2021-10-18 18:53 [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening Sergey Kaplun via Tarantool-patches
  2021-10-27 16:08 ` sergos via Tarantool-patches
@ 2022-06-28 13:21 ` Igor Munkin via Tarantool-patches
  2022-06-30 12:09 ` Igor Munkin via Tarantool-patches
  2 siblings, 0 replies; 7+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-28 13:21 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for the patch! LGTM, considering all fixes after Sergos review.

-- 
Best regards,
IM

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

* Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening.
  2021-10-18 18:53 [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening Sergey Kaplun via Tarantool-patches
  2021-10-27 16:08 ` sergos via Tarantool-patches
  2022-06-28 13:21 ` Igor Munkin via Tarantool-patches
@ 2022-06-30 12:09 ` Igor Munkin via Tarantool-patches
  2 siblings, 0 replies; 7+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-30 12: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 master, 2.10 and 1.10.

On 18.10.21, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> Reported by Matthew Burk.
> 
> (cherry picked from commit 9f0caad0e43f97a4613850b3874b851cb1bc301d)
> 
> The simplify_conv_sext optimization is used for reduction of widening.
> cdata indexing narrow optimization uses it for narrowing of a C array
> index. The optimization eliminates sign extension for corresponding
> integer value. However, this conversion cannot be omitted for non
> constant values (for example loading stack slots) as far as their sign
> extension may change. The emitted machine code may be incorrect without
> aforementioned conversion (for example mov instruction instead movsxd is
> used on x86 architecture). As a result the value in a destination
> register during trace execution is invalid.
> 
> This patch allows this optimization only for constant integer values.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> ---
> 
> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-fold-simplify-conv-sext
> 
>  src/lj_opt_fold.c                             |  2 +-
>  .../lj-fix-fold-simplify-conv-sext.test.lua   | 35 +++++++++++++++++++
>  2 files changed, 36 insertions(+), 1 deletion(-)
>  create mode 100644 test/tarantool-tests/lj-fix-fold-simplify-conv-sext.test.lua
> 

<snipped>

> -- 
> 2.31.0
> 

-- 
Best regards,
IM

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

end of thread, other threads:[~2022-06-30 12:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 18:53 [Tarantool-patches] [PATCH luajit] Fix FOLD rule for strength reduction of widening Sergey Kaplun via Tarantool-patches
2021-10-27 16:08 ` sergos via Tarantool-patches
2021-10-28 11:36   ` Sergey Kaplun via Tarantool-patches
2021-10-28 12:37     ` sergos via Tarantool-patches
2021-10-29  7:49       ` Sergey Kaplun via Tarantool-patches
2022-06-28 13:21 ` Igor Munkin via Tarantool-patches
2022-06-30 12: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