Re: [Tarantool-patches] [PATCH luajit 1/2] Cleanup CPU detection and tuning for old CPUs.

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

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

Hi, Sergey,

Thanks for the fixes! LGTM

On 15.01.2025 16:10, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Updated the commit message and force-pushed the branch.
>
> On 14.01.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch!
>>
>> LGTM with a minor comment
>>
>> Sergey
>>
>> On 13.01.2025 18:17, Sergey Kaplun wrote:
>>> From: Mike Pall <mike>
>>>
>>> (cherry picked from commit 0eddcbead2d67c16dcd4039a6765b9d2fc8ea631)
>>>
>>> This patch does the following refactoring:
>>> 1) Drops optimizations for the Intel Atom CPU [1]: removes the
>>>      `JIT_F_LEA_AGU` flag and related optimizations. The considerations
>>>      for the use of LEA are complex and very CPU-specific, mostly
>>>      dependent on the number of operands. Mostly, it isn't worth it due to
>>>      the extra register pressure and/or extra instructions.
>> I would say explicitly that `JIT_F_LEA_AGU` is used in "Well, yes, that
>> applies to the original and obsolete Atom architecture. Today "Intel
>> Atom" is just a trade name for reduced-performance implementations of
>> the current Intel architecture."
>>
>> as Mike explained in LUAJIT#24. So there are no any risks for tarantool
>> users
>>
>> regarding performance degradation.
> Added, as you suggested. The new commit message is the following:
>
> | Cleanup CPU detection and tuning for old CPUs.
> |
> | (cherry picked from commit 0eddcbead2d67c16dcd4039a6765b9d2fc8ea631)
> |
> | This patch does the following refactoring:
> | 1) Drops optimizations for the Intel Atom CPU [1]: removes the
> |    `JIT_F_LEA_AGU` flag and related optimizations. The considerations
> |    for the use of LEA are complex and very CPU-specific, mostly
> |    dependent on the number of operands. Mostly, it isn't worth it due to
> |    the extra register pressure and/or extra instructions.
> |    Be aware that it applies to the original and obsolete Atom
> |    architecture. Today "Intel Atom" is just a trade name for
> |    reduced-performance implementations of the current Intel
> |    architecture.
> | 2) Drops optimizations for the AMD K8, K10 CPU [2][3]: removes the
> |    `JIT_F_PREFER_IMUL` flag and related optimizations.
> | 3) Refactors JIT flags defined in the <lj_jit.h>. Now all CPU-specific
> |    JIT flags are defined as the left shift of `JIT_F_CPU` instead of
> |    hardcoded constants, similar for the optimization flags.
> | 4) Adds detection of the ARM8 CPU.
> | 5) Drops the check for SSE2 since the VM already presumes CPU supports
> |    it.
> | 6) Adds checks for `__ARM_ARCH`[4] macro in <lj_arch.h>.
> | 7) Drops outdated comment in the amalgamation file about memory
> |    requirements.
> |
> | Sergey Kaplun:
> | * added the description for the patch
> |
> | [1]:https://en.wikipedia.org/wiki/Intel_Atom
> | [2]:https://en.wikipedia.org/wiki/AMD_K8
> | [3]:https://en.wikipedia.org/wiki/AMD_K10
> | [4]:https://developer.arm.com/documentation/dui0774/l/Other-Compiler-specific-Features/Predefined-macros
> |
> | Part of tarantool/tarantool#10709
>
>>> 2) Drops optimizations for the AMD K8, K10 CPU [2][3]: removes the
>>>      `JIT_F_PREFER_IMUL` flag and related optimizations.
>>> 3) Refactors JIT flags defined in the <lj_jit.h>. Now all CPU-specific
>>>      JIT flags are defined as the left shift of `JIT_F_CPU` instead of
>>>      hardcoded constants, similar for the optimization flags.
>>> 4) Adds detection of the ARM8 CPU.
>>> 5) Drops the check for SSE2 since the VM already presumes CPU supports
>>>      it.
>>> 6) Adds checks for `__ARM_ARCH`[4] macro in <lj_arch.h>.
>>> 7) Drops outdated comment in the amalgamation file about memory
>>>      requirements.
>>>
>>> Sergey Kaplun:
>>> * added the description for the patch
>>>
>>> [1]:https://en.wikipedia.org/wiki/Intel_Atom
>>> [2]:https://en.wikipedia.org/wiki/AMD_K8
>>> [3]:https://en.wikipedia.org/wiki/AMD_K10
>>> [4]:https://developer.arm.com/documentation/dui0774/l/Other-Compiler-specific-Features/Predefined-macros
>>>
>>> Part of tarantool/tarantool#10709
>>> ---
> <snipped>
>

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