From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 53BBFB41B81; Fri, 12 Apr 2024 14:23:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 53BBFB41B81 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1712921020; bh=Cbzyk3I91apY425/K/YTQH61dejvlSgqAkBBRiUSrB0=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=L0cqXNu6VBEcsICNokK4Q03YJDxJN+Jd7d3BA7BNbdVsSNkaNOjw+53wnvm46kLqY HAvjFWG35utj1IbRaPtwgs8vMVhgUeX045RXaubvHHEbPDgy8rm1f5FwvNH9DGh915 dHucZMIYQw+5iLjESFxM6MRXgSyn3j/zajgMl95c= Received: from smtp17.i.mail.ru (smtp17.i.mail.ru [95.163.41.70]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 412D5B41B81 for ; Fri, 12 Apr 2024 14:23:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 412D5B41B81 Received: by smtp17.i.mail.ru with esmtpa (envelope-from ) id 1rvF0L-00000006upE-0iTH; Fri, 12 Apr 2024 14:23:37 +0300 Date: Fri, 12 Apr 2024 14:19:35 +0300 To: Sergey Bronnikov Message-ID: References: <7bdffd2650a785877e03584e6d532e855d09de8a.1712841312.git.sergeyb@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7bdffd2650a785877e03584e6d532e855d09de8a.1712841312.git.sergeyb@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD9D327C87852EB66D3F2DF8DAE835AF018F93A0B391F16AD37182A05F5380850401683BBE6BD22E2CB2EB5D77EF37489D155F56FEEF24848B26B69C249257E1AB1AC22AB14350DA595 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A4C4638C9DDF45FCEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063763305FB9D3ECC1898638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D817F49A8ABA37FB3A57E361BE7CB7F03992DAD78E0935A372CC7F00164DA146DAFE8445B8C89999728AA50765F79006374F09588CB15B21E6389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8FA486DC37A503D0BF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE33AC447995A7AD189C87C2358C8A9BAF3A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7356436AE5DD6441DC7C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5DF87A99BDEBC22915002B1117B3ED69646A068FA7CDCBE06C638DF663A625AFA823CB91A9FED034534781492E4B8EEAD47A3109F1ACFD409BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFA98D4098C8411C0109905BE415937AA2210C2611DEB5B05C1C09EE5286698E2F192AF36DABE858451E5F415AA55898CC8D510BED22671B0B285EC9AAA5C55A0FDFD1BFB854695DCE5F4332CA8FE04980913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojj8Sl06kWf01Mm0rslOIsVw== X-DA7885C5: 85C0E3C4A0D403B4F255D290C0D534F950D5FBD3297CD9B50E59E4C08EBD2C0AF5BCB6F3DAAC02B5D58A62FD7D354DCE90A62EE20EBFBCA6DE9604509FAD3A5C X-Mailru-Sender: 689FA8AB762F7393590D8C940224AE33B8E025EB009CB9D5327C6AA2D5D0C180D625AA5538BC52B5E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/4][v2] OSX/iOS/ARM64: Fix generation of Mach-O object files. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the fixes! LGTM after fixing a few minor nits below. On 11.04.24, Sergey Bronnikov wrote: > From: Mike Pall > > --- > src/jit/bcsave.lua | 6 +- > test/LuaJIT-tests/CMakeLists.txt | 9 + > ...-865-cross-generation-mach-o-file.test.lua | 300 ++++++++++++++++++ > 3 files changed, 312 insertions(+), 3 deletions(-) > create mode 100644 test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua > > diff --git a/src/jit/bcsave.lua b/src/jit/bcsave.lua > index a287d675..7aec1555 100644 > --- a/src/jit/bcsave.lua > +++ b/src/jit/bcsave.lua > diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt > index b8e4dfc4..6d073700 100644 > --- a/test/LuaJIT-tests/CMakeLists.txt > +++ b/test/LuaJIT-tests/CMakeLists.txt > @@ -52,6 +52,15 @@ if(LUAJIT_NO_UNWIND) > set(LUAJIT_TEST_TAGS_EXTRA +internal_unwinder) > endif() > > +if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512") > + # FIXME: Test verifies bitwise operations on numbers. Nit: comment line width is more than 66 symbols. > + # There is a known issue - bitop doesn't work in LuaJIT built > + # with the enabled AVX512 instruction set, see > + # https://github.com/tarantool/tarantool/issues/6787. > + # Hence, skip this when "skylake-avx512" is passed. > + set(LUAJIT_TEST_TAGS_EXTRA +avx512) > +endif() > + Should this be a part of the first commit? > set(TEST_SUITE_NAME "LuaJIT-tests") > > # XXX: The call produces both test and target > diff --git a/test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua b/test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua > new file mode 100644 > index 00000000..04fb5495 > --- /dev/null > +++ b/test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua > @@ -0,0 +1,300 @@ > +local tap = require('tap') > +local test = tap.test('lj-865-cross-generation-mach-o-file') > +local utils = require('utils') > + > +test:plan(1) > + > +-- The test creates an object file in Mach-O format with LuaJIT > +-- bytecode and checks the validity of the object file fields. > +-- > +-- The original problem is reproduced with LuaJIT that built with Typo: s/LuaJIT that built/LuaJIT, which is built/ > +-- enabled AVX512F instructions. The support for AVX512F could be > +-- checked in `/proc/cpuinfo` on Linux and > +-- `sysctl hw.optional.avx512f` on Mac. AVX512F must be > +-- implicitly enabled in a C compiler by passing a CPU codename. > +-- Please take a look at the GCC Online Documentation [1] for > +-- available CPU codenames. Also, see the Wikipedia for CPUs with > +-- AVX-512 support [2]. > +-- To detect the CPU codename execute: Typo: s/codename/codename,/ > +-- `gcc -march=native -Q --help=target | grep march`. > +-- > +-- 1. https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html > +-- 2. https://en.wikipedia.org/wiki/AVX-512#CPUs_with_AVX-512 > +-- > +-- Manual steps for reproducing are the following: > +-- > +-- $ CC=gcc TARGET_CFLAGS='skylake-avx512' cmake -S . -B build > +-- $ cmake --build build --parallel > +-- $ echo > test.lua > +-- $ LUA_PATH="src/?.lua;;" luajit -b -o osx -a arm test.lua test.o > +-- $ file test.o > +-- empty.o: DOS executable (block device driver) > + > +local ffi = require('ffi') Nit: Why do we require the ffi here alongside from others requires? > + > +-- LuaJIT can generate so called Universal Binary with Lua > +-- > +-- There are a good visual representation of Universal Binary Typo: s/are/is/ > +-- in "Mac OS X Internals" book (pages 67-68) [5] and in the [6]. > +-- Below is the schematic structure of Universal Binary, which > +-- includes two executables for PowerPC and Intel i386 (omitted): > +-- > +-- 0x0000000 --------------------------------------- > +local function create_obj_file(name, arch) > + local mach_o_path = os.tmpname() .. '.o' > + local lua_path = os.getenv('LUA_PATH') > + local lua_bin = utils.exec.luacmd(arg):match('%S+') > + local cmd_fmt = 'LUA_PATH="%s" %s -b -n "%s" -o osx -a %s -e "print()" %s' > + local cmd = (cmd_fmt):format(lua_path, lua_bin, name, arch, mach_o_path) Nit: Typo: s/(cmd_fmt)/cmd_fmt/ > + local ret = os.execute(cmd) > + assert(ret == 0, 'cannot create an object file') > + return mach_o_path > +end > + > +-- Parses a buffer in the Mach-O format and returns Nit: The comment line looks underfilled. > +-- the FAT magic number and `nfat_arch`. > +local function read_mach_o(buf) > +local SUM_CPUTYPE = { Minor: It will be nice to add the comment: | -- x86 + arm. > + arm = 7 + 12, > +} > +local SUM_CPUSUBTYPE = { Minor: It will be nice to add the comment: | -- x86 + arm. > + arm = 3 + 9, > +} > + > +local function build_and_check_mach_o(subtest, hw_arch) > + assert(hw_arch == 'arm') > + > + subtest:plan(4) > +test:test('arm', build_and_check_mach_o, 'arm') Minor: we can use `subtest.name` as the definition of the `hw_arch` in the `build_and_check_mach_o()`, so it helps to avoid duplication of arch usage. Matter of taste. Feel free to ignore. > + > +test:done(true) > -- > 2.34.1 > -- Best regards, Sergey Kaplun