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

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

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>

-- 
Best regards,
Sergey Kaplun