Re: [Tarantool-patches] [PATCH luajit 3/3] FFI/ARM64/OSX: Fix vararg call handling.

[Sergey Ostanevich via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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



> On 13 May 2021, at 13:44, Igor Munkin <imun@tarantool.org> wrote:
> 
> Sergos,
> 
> On 13.05.21, Sergey Ostanevich wrote:
>> Well, this doesn't help me with
>> 
>>> Since all LJ_TARGET_IOS usage is done with LJ_TARGET_ARM64 define being
>>> set, we can simply replace all occurrences with LJ_TARGET_OSX.
> 
> OK, as a result of offline discussion I've finally got it (hope you
> too). Everything written above relates only to FFI sources that are
> touched within this changeset (consider FFI prefix in commit subject).
> 
>> 
>> alongside with
>> 
>>> src/lj_prng.c:112:#if LJ_TARGET_OSX && !LJ_TARGET_IOS
> 
> Strictly saying, this commit[1] has not been backported yet.
> 
>> 
>> because if we apply first then the second will evaluate into
>> 
>> src/lj_prng.c:112:#if LJ_TARGET_OSX && !LJ_TARGET_OSX
>> 
>> which is always false?
>> 
>> Also there are still operable 32bit apps, including games, so iOS
>> still supports 32bits - can this change cause problems? I believe Mike
>> doesn’t care too much, so it can easily slip through.
> 
> As we discussed, 32-bit platforms are not touched in this commit, so
> nothing criminal.
> 
> Anyway, I see this is unclear to you, but everything is OK for Sergey.
> Do I need to reword this part of commit message or even drop it?

Just update the wording

All LJ_TARGET_IOS uses in FFI sources are done with LJ_TARGET_ARM64 define being
set, so we can simply replace these occurrences with LJ_TARGET_OSX.

And it’s LGTM.

Sergos
> 
>> 
>> Sergos
>> 
>> 
>> 
>>> On 13 May 2021, at 00:59, Igor Munkin <imun@tarantool.org> wrote:
>>> 
>>> Sergos,
>>> 
>>> On 12.05.21, Sergey Ostanevich wrote:
>>>> Hi!
>>>> 
>>>> I can’t get this then
>>>> 
>>>> src/lj_prng.c:112:#if LJ_TARGET_OSX && !LJ_TARGET_IOS
>>> 
>>> There is a verbose comment nearby[1] and the corresponding issue[2].
>>> 
>>>> 
>>>> How’s that survived in 2.1 branch?
>>>> 
>>>> regards,
>>>> Sergos
>>>> 
>>>> 
>>>>> On 11 May 2021, at 14:31, Igor Munkin <imun@tarantool.org> wrote:
>>>>> 
>>>>> Sergey,
>>>>> 
>>>>> Thanks for your review!
>>>>> 
>>>>> On 11.05.21, Sergey Kaplun wrote:
>>>>>> Hi, Igor!
>>>>>> 
>>>>>> Thanks for the patch!
>>>>>> LGTM!
>>>>> 
>>>>> Added your tag:
>>>>> | Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
>>>>> 
>>>>> Also mentioned the issue[1]:
>>>>> | Resolves tarantool/tarantool#6066
>>>>> 
>>>>>> 
>>>>>> -- 
>>>>>> Best regards,
>>>>>> Sergey Kaplun
>>>>> 
>>>>> [1]: https://github.com/tarantool/tarantool/issues/6066
>>>>> 
>>>>> -- 
>>>>> Best regards,
>>>>> IM
>>>> 
>>> 
>>> [1]: https://github.com/LuaJIT/LuaJIT/blob/v2.1/src/lj_prng.c#L113-L119
>>> [2]: https://github.com/LuaJIT/LuaJIT/issues/668
>>> 
>>> -- 
>>> Best regards,
>>> IM
>> 
> 
> [1]: https://github.com/LuaJIT/LuaJIT/commit/7877369 <https://github.com/LuaJIT/LuaJIT/commit/7877369>
> 
> -- 
> Best regards,
> IM


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