> On 13 May 2021, at 13:44, Igor Munkin 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 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 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 >>>>> >>>>> 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 > > -- > Best regards, > IM