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 8839F45FC07; Mon, 15 May 2023 12:18:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8839F45FC07 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1684142310; bh=C3Ux9Y72HZZaC74Gb+qjy2e8t03xRrYDCBYIInUtVGw=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=kpx6+uzsN93I+bzDdIMnTSX9C9VLC480RlhTTFArr+f1AFEEc585DK3MtzCtvluCv kgDCcypjd6iXS3JPu7ZvIXdnQmYnD8K9jQJur4vRtiB+5ZW50WTwGn2EfMb6cXLn94 Cc7l1JvmxK7HrQVAynVc8GfbKnP+1y5Rx+ahR/sM= Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 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 C9C2345FC0B for ; Mon, 15 May 2023 12:16:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C9C2345FC0B Received: by mail-lf1-f44.google.com with SMTP id 2adb3069b0e04-4f139de8cefso63936907e87.0 for ; Mon, 15 May 2023 02:16:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684142206; x=1686734206; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4LqEAE3erCwVzKSg6Lfnpsa2Nhg0ozPCKtKacNfNP4E=; b=ZM9g9Pfk5OLITJOkVGzBAUotAhTsw/caKumfxw6rFkO3mKn5Rwv0Pw0WhPoAO73x7/ 4Bcio/KarnxyWP+8TM/5VpkfHmy6WIoLvVfvbabt2Oz3Pu95UvAY70d5YKPpyPvYzcRM Sr0xOjg4j36/MMy6p+OWfkwJADQhH7CJOvkeZRV2/fV5Yh3UB2nVUsi3/9TQOeTKVWKa R+vhJCYYjaUqB3Tdjo7+Zo06+bMfFKMH0o1sl7D/l3ZW8uYNm3QfZCEgLkzE04elfHIv i3lZJiXPUFfnFv6vYfkbGjXplxkrROK75RgEiu/Q9bc+u54YjeJs8DE3GevDXvO4kxZ9 6+sw== X-Gm-Message-State: AC+VfDwt3Ff99mIr0xGIgD00SRJGEuWIa/VWqXYuICIzf2D9Sl+Lji9G MpCrcGz44wxvTzRReokNKhXZ3HaGIN6eVrp2 X-Google-Smtp-Source: ACHHUZ5uSPnfRR9Fcflip2G4rje4pVFOlJk56tgm7nyUWfaaRAQaWyzzGjTTrbZp5w5W5cFFuZiRNA== X-Received: by 2002:a2e:855a:0:b0:2a9:fa39:235e with SMTP id u26-20020a2e855a000000b002a9fa39235emr6203186ljj.26.1684142205565; Mon, 15 May 2023 02:16:45 -0700 (PDT) Received: from localhost.localdomain ([185.205.79.32]) by smtp.gmail.com with ESMTPSA id a4-20020a2e88c4000000b002ac7c9d2806sm3739607ljk.50.2023.05.15.02.16.44 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Mon, 15 May 2023 02:16:45 -0700 (PDT) To: tarantool-patches@dev.tarantool.org, sergos@tarantool.org, imun@tarantool.org, skaplun@tarantool.org, m.kokryashkin@tarantool.org Date: Mon, 15 May 2023 12:16:22 +0300 Message-Id: <20230515091622.30232-5-max.kokryashkin@gmail.com> X-Mailer: git-send-email 2.39.2 (Apple Git-143) In-Reply-To: <20230515091622.30232-1-max.kokryashkin@gmail.com> References: <20230515091622.30232-1-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit v4 4/4] Fix IR_RENAME snapshot number. Follow-up fix for a32aeadc. 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: Maksim Kokryashkin via Tarantool-patches Reply-To: Maksim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Reported by Victor Bombi, analyzed by XmiliaH. Thanks! (cherry-picked from commit bf51d3535109c4745bfbbe19a5587a9eac00259a) If the `snapalloc` flag is set, then the allocation has not occurred yet, meaning that rename is applied to the next snapshot. Otherwise, refs are already allocated and rename is applied to the current snapshot. Maxim Kokryashkin: * added the description and the test for the problem Part of tarantool/tarantool#7745 Part of tarantool/tarantool#8069 --- src/lj_asm.c | 9 ++- .../lj-688-snap-ir-rename.test.lua | 60 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/lj-688-snap-ir-rename.test.lua diff --git a/src/lj_asm.c b/src/lj_asm.c index f7c40fea..9bbfb2bf 100644 --- a/src/lj_asm.c +++ b/src/lj_asm.c @@ -682,7 +682,14 @@ static void ra_rename(ASMState *as, Reg down, Reg up) RA_DBGX((as, "rename $f $r $r", regcost_ref(as->cost[up]), down, up)); emit_movrr(as, ir, down, up); /* Backwards codegen needs inverse move. */ if (!ra_hasspill(IR(ref)->s)) { /* Add the rename to the IR. */ - ra_addrename(as, down, ref, as->snapno); + /* + ** The rename is effective at the subsequent (already emitted) exit + ** branch. This is for the current snapshot (as->snapno). Except if we + ** haven't yet allocated any refs for the snapshot (as->snapalloc == 1), + ** then it belongs to the next snapshot. + ** See also the discussion at asm_snap_checkrename(). + */ + ra_addrename(as, down, ref, as->snapno + as->snapalloc); } } diff --git a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua new file mode 100644 index 00000000..f626cfb6 --- /dev/null +++ b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua @@ -0,0 +1,60 @@ +local tap = require('tap') +local test = tap.test('lj-688-snap-ir-rename'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) +test:plan(1) + +jit.opt.start('hotloop=1') + +-- IR for the loop below looks like the following +-- before the patch: +-- +-- 0021 ------------ LOOP ------------ +-- .... SNAP #6 [ ---- ---- 0019 false 0007 ] +-- 0022 > num NE 0019 +1.1 +-- .... SNAP #7 [ ---- ---- 0019 false 0007 ] +-- 0023 rbp > int CONV 0019 int.num +-- 0024 > int ABC 0013 0023 +-- 0025 p32 AREF 0015 0023 +-- 0026 xmm6 > num ALOAD 0025 +-- .... SNAP #8 [ ---- ---- 0019 false ---- ] +-- 0027 > num ULE 0026 +0 +-- 0028 xmm7 + num ADD 0019 +1 +-- .... SNAP #9 [ ---- ---- 0019 false ] +-- 0029 > num LT 0028 +5 +-- 0030 xmm7 num PHI 0019 0028 +-- 0031 xmm6 nil RENAME 0019 #8 +-- ---- TRACE 1 stop -> loop +-- +-- RENAME instruction is applied to the 8th snapshot, when it +-- should be applied to the 9th. After the patch that line looks +-- the following way: +-- +-- 0031 xmm6 nil RENAME 0019 #9 + +-- XXX: Note, that reproducer below won't fail on ARM/ARM64 +-- even before the patch, because `IR_RENAME` is not emitted +-- for any of the instructions produced. +-- Although it is possible to achieve the same faulty behavior +-- before the patch on ARM/ARM64, it requires a more complex +-- reproducer. Since the code affected by the patch is +-- platform-agnostic, there is no real necessity to test it +-- against ARM/ARM64 separately. +-- +-- See also https://drive.google.com/file/d/1iYkFx3F0DOtB9o9ykWfCEm-OdlJNCCL0/view?usp=share_link + + +local vals = {-0.1, 0.1, -0.1, 0.1} +local i = 1 +local _ +while i < 5 do + assert(i ~= 1.1) + local l1 = vals[i] + _ = l1 > 0 + i = i + 1 +end + +test:ok(true, 'IR_RENAME is fine') +-- `test:check() and 0 or 1` is replaced with just test:check() +-- here, because otherwise, it affects the renaming process. +os.exit(test:check()) -- 2.39.2 (Apple Git-143)