From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4C52E469719 for ; Wed, 14 Oct 2020 22:32:52 +0300 (MSK) Date: Wed, 14 Oct 2020 22:22:14 +0300 From: Igor Munkin Message-ID: <20201014192214.GG18920@tarantool.org> References: <20201014190438.GA9302@root> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20201014190438.GA9302@root> Subject: Re: [Tarantool-patches] [PATCH] jit: fix cdatanum addressing for GC64 mode on x86 List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Sergey, Thanks for your review! On 14.10.20, Sergey Kaplun wrote: > Hi! > > Thanks for the patch! > I have only one nitpick/question regarding to commit message. > > Otherwise, LGTM. Added your tag: | Reviewed-by: Sergey Kaplun > > On 14.10.20, Igor Munkin wrote: > > This patch fixes the regression introduced in scope of > > 5f6775ae0e141422193ad9b492806834064027ca ('core: introduce various > > platform metrics'). As a result of the patch displacement is > > misencoded when GC64 mode is enabled. > > > > In X86 long mode 32-bit displacement is encoded either via SIB byte or > > is addressed relatively to RIP register value. The first approach is > > used in JIT for 32-bit addresses (i.e. when GC64 mode is disabled), but > > doesn't work for 64-bit ones. As a result all addresses to GG_State > > contents to be "hardcoded" on the trace are encoded relatively to > > RID_DISPATCH register (i.e. callee-safe R14 register) containing global > > dispatch table. For this purpose this register is not used by the JIT > > register allocator in GC64 build and not spoiled throughout LuaJIT VM > > cycle (and therefore trace execution). > > > > NB: Since R14 is the additional GRP, the instruction ought to be > > REX-prefixed. > > Nit: Also REX prefix is needed to specify 64-bit operand size, isn't > it? Nice catch! Totally missed W-bit. Adjusted this line the following way: | NB: Since R14 is the additional GRP and default operand size is | 32-bit (but 64-bit is used), the instruction ought to be REX-prefixed. > > > > > Follows up tarantool/tarantool#5187 > > > > Reported-by: Vladislav Shpilevoy > > Signed-off-by: Igor Munkin > > --- > > > > Branch: https://github.com/tarantool/luajit/compare/imun/gh-5187-fix-disp-encoding-on-gc64 > > > > -- > Best regards, > Sergey Kaplun -- Best regards, IM