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 8C7616ECEA; Wed, 30 Mar 2022 02:49:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8C7616ECEA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1648597771; bh=22sEqzldBnmkMFsFNQ76JGwsENKnDBuz/+l9iVkmYZM=; 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=Haccf7P+uwt7lcwhNGFVQAu7iYVIPqS63vb0vv48kXALz6WIxeZRnfSSObj4yLfV/ sRQTs/UkCXjFfKQT/cHgokK1Od8uUohxwS7ygJnknXm+EiKsxZf9kWV3m171QqTerY 97laoUBdqbSy/lutVYGppqo7FyE65xSYWFIDtL8I= Received: from mail-lf1-f45.google.com (mail-lf1-f45.google.com [209.85.167.45]) (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 4456B6ECEA for ; Wed, 30 Mar 2022 02:49:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4456B6ECEA Received: by mail-lf1-f45.google.com with SMTP id e16so32929377lfc.13 for ; Tue, 29 Mar 2022 16:49:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Qq4G7gcYMNr7VGWdNq2Btop2jQ1uYOv8waHwauQAnGA=; b=fjNXA9aQVQ2XwGyO3UkRO9lnc84LXT9wZiCv90rtjRwWnzTTN49Fhtj9NNI/y8/TKc ykpru/Bv7o9UJPEmhVbFE6JmYDUIvHzREDEzYOyVrLss+wBe+4etXDdqr5G8JGvnPshO n9ovgu/t4yE2wdYT0lmqmt2G9M/MG51vHTwYoub85OJsDj4curwcywi6hkj7IT6+pBwJ M9gmN7e5Gf5eYdEDdtJrF1+kfl08u/sArFPC/a2Dhbvzi2jHDsuyohh87L4uJyyCgMZH CR4RkRPEhfjj1HKLgY+r11I/HB6i64GBLf+6yTLvUccPb0Jy4FdUAwBrQj+TAE1TBuzg m+Qw== X-Gm-Message-State: AOAM530q46962Ya8ic2DpM9u240wUD90magjJUgr5h9PPGm+LJFS+3Jp rhWBB7iWcru8YxEQGO66Yo/5v71E0V8uTg== X-Google-Smtp-Source: ABdhPJyozXZmLVibSt28C0khjEAFT0BW7E4cMFOmuX2zdzp6cff3vhymaTw58Z+LMfbNoCAYz5e/Gg== X-Received: by 2002:a05:6512:3192:b0:44a:b43f:d58f with SMTP id i18-20020a056512319200b0044ab43fd58fmr4491521lfe.179.1648597747336; Tue, 29 Mar 2022 16:49:07 -0700 (PDT) Received: from localhost.localdomain ([93.175.11.199]) by smtp.gmail.com with ESMTPSA id k10-20020a2eb74a000000b0024965dbf2d8sm2236021ljo.82.2022.03.29.16.49.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Mar 2022 16:49:07 -0700 (PDT) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, imun@tarantool.org, skaplun@tarantool.org Date: Wed, 30 Mar 2022 02:48:54 +0300 Message-Id: X-Mailer: git-send-email 2.35.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit v4 1/3] Fix math.min()/math.max() inconsistencies. 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall math.min()/math.max() could produce different results when compiled. Previously, these functions did not check for the number of arguments, which in some situations led to incorrect behavior. Consider this example: ``` jit.opt.start(0, "hotloop=1") local r, msg = pcall(function () math.min() end) r, msg = pcall(function () math.min() end) ``` Before this patch, this snippet of code completed successfully, although it was not supposed to, since there were no args to math.min(). This patch adds check for the number of arguments for jit-compiled version of math.min/max, and it also adds the corresponding test case for the mentioned issue. Part of tarantool/tarantool#6163 Part of tarantool/tarantool#6548 --- src/lj_asm_arm.h | 6 +-- src/lj_asm_arm64.h | 6 +-- src/lj_opt_fold.c | 53 +++++++------------ src/lj_vmmath.c | 4 +- src/vm_arm.dasc | 4 +- src/vm_arm64.dasc | 4 +- src/vm_x64.dasc | 2 +- src/vm_x86.dasc | 2 +- .../gh-6163-jit-min-max.test.lua | 20 +++++++ 9 files changed, 53 insertions(+), 48 deletions(-) create mode 100644 test/tarantool-tests/gh-6163-jit-min-max.test.lua diff --git a/src/lj_asm_arm.h b/src/lj_asm_arm.h index 4fd08b9e..84ab06c0 100644 --- a/src/lj_asm_arm.h +++ b/src/lj_asm_arm.h @@ -1664,8 +1664,8 @@ static void asm_min_max(ASMState *as, IRIns *ir, int cc, int fcc) asm_intmin_max(as, ir, cc); } -#define asm_min(as, ir) asm_min_max(as, ir, CC_GT, CC_HI) -#define asm_max(as, ir) asm_min_max(as, ir, CC_LT, CC_LO) +#define asm_min(as, ir) asm_min_max(as, ir, CC_GT, CC_PL) +#define asm_max(as, ir) asm_min_max(as, ir, CC_LT, CC_LE) /* -- Comparisons --------------------------------------------------------- */ @@ -1857,7 +1857,7 @@ static void asm_hiop(ASMState *as, IRIns *ir) } else if ((ir-1)->o == IR_MIN || (ir-1)->o == IR_MAX) { as->curins--; /* Always skip the loword min/max. */ if (uselo || usehi) - asm_sfpmin_max(as, ir-1, (ir-1)->o == IR_MIN ? CC_HI : CC_LO); + asm_sfpmin_max(as, ir-1, (ir-1)->o == IR_MIN ? CC_PL : CC_LE); return; #elif LJ_HASFFI } else if ((ir-1)->o == IR_CONV) { diff --git a/src/lj_asm_arm64.h b/src/lj_asm_arm64.h index cc8c0c9d..e93e29e3 100644 --- a/src/lj_asm_arm64.h +++ b/src/lj_asm_arm64.h @@ -1594,7 +1594,7 @@ static void asm_fpmin_max(ASMState *as, IRIns *ir, A64CC fcc) Reg dest = (ra_dest(as, ir, RSET_FPR) & 31); Reg right, left = ra_alloc2(as, ir, RSET_FPR); right = ((left >> 8) & 31); left &= 31; - emit_dnm(as, A64I_FCSELd | A64F_CC(fcc), dest, left, right); + emit_dnm(as, A64I_FCSELd | A64F_CC(fcc), dest, right, left); emit_nm(as, A64I_FCMPd, left, right); } @@ -1606,8 +1606,8 @@ static void asm_min_max(ASMState *as, IRIns *ir, A64CC cc, A64CC fcc) asm_intmin_max(as, ir, cc); } -#define asm_max(as, ir) asm_min_max(as, ir, CC_GT, CC_HI) -#define asm_min(as, ir) asm_min_max(as, ir, CC_LT, CC_LO) +#define asm_min(as, ir) asm_min_max(as, ir, CC_LT, CC_PL) +#define asm_max(as, ir) asm_min_max(as, ir, CC_GT, CC_LE) /* -- Comparisons --------------------------------------------------------- */ diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c index 3c508062..29b760b4 100644 --- a/src/lj_opt_fold.c +++ b/src/lj_opt_fold.c @@ -1774,8 +1774,6 @@ LJFOLDF(reassoc_intarith_k64) #endif } -LJFOLD(MIN MIN any) -LJFOLD(MAX MAX any) LJFOLD(BAND BAND any) LJFOLD(BOR BOR any) LJFOLDF(reassoc_dup) @@ -1785,6 +1783,15 @@ LJFOLDF(reassoc_dup) return NEXTFOLD; } +LJFOLD(MIN MIN any) +LJFOLD(MAX MAX any) +LJFOLDF(reassoc_dup_minmax) +{ + if (fins->op2 == fleft->op2) + return LEFTFOLD; /* (a o b) o b ==> a o b */ + return NEXTFOLD; +} + LJFOLD(BXOR BXOR any) LJFOLDF(reassoc_bxor) { @@ -1823,23 +1830,12 @@ LJFOLDF(reassoc_shift) return NEXTFOLD; } -LJFOLD(MIN MIN KNUM) -LJFOLD(MAX MAX KNUM) LJFOLD(MIN MIN KINT) LJFOLD(MAX MAX KINT) LJFOLDF(reassoc_minmax_k) { IRIns *irk = IR(fleft->op2); - if (irk->o == IR_KNUM) { - lua_Number a = ir_knum(irk)->n; - lua_Number y = lj_vm_foldarith(a, knumright, fins->o - IR_ADD); - if (a == y) /* (x o k1) o k2 ==> x o k1, if (k1 o k2) == k1. */ - return LEFTFOLD; - PHIBARRIER(fleft); - fins->op1 = fleft->op1; - fins->op2 = (IRRef1)lj_ir_knum(J, y); - return RETRYFOLD; /* (x o k1) o k2 ==> x o (k1 o k2) */ - } else if (irk->o == IR_KINT) { + if (irk->o == IR_KINT) { int32_t a = irk->i; int32_t y = kfold_intop(a, fright->i, fins->o); if (a == y) /* (x o k1) o k2 ==> x o k1, if (k1 o k2) == k1. */ @@ -1852,24 +1848,6 @@ LJFOLDF(reassoc_minmax_k) return NEXTFOLD; } -LJFOLD(MIN MAX any) -LJFOLD(MAX MIN any) -LJFOLDF(reassoc_minmax_left) -{ - if (fins->op2 == fleft->op1 || fins->op2 == fleft->op2) - return RIGHTFOLD; /* (b o1 a) o2 b ==> b; (a o1 b) o2 b ==> b */ - return NEXTFOLD; -} - -LJFOLD(MIN any MAX) -LJFOLD(MAX any MIN) -LJFOLDF(reassoc_minmax_right) -{ - if (fins->op1 == fright->op1 || fins->op1 == fright->op2) - return LEFTFOLD; /* a o2 (a o1 b) ==> a; a o2 (b o1 a) ==> a */ - return NEXTFOLD; -} - /* -- Array bounds check elimination -------------------------------------- */ /* Eliminate ABC across PHIs to handle t[i-1] forwarding case. @@ -1995,8 +1973,6 @@ LJFOLDF(comm_comp) LJFOLD(BAND any any) LJFOLD(BOR any any) -LJFOLD(MIN any any) -LJFOLD(MAX any any) LJFOLDF(comm_dup) { if (fins->op1 == fins->op2) /* x o x ==> x */ @@ -2004,6 +1980,15 @@ LJFOLDF(comm_dup) return fold_comm_swap(J); } +LJFOLD(MIN any any) +LJFOLD(MAX any any) +LJFOLDF(comm_dup_minmax) +{ + if (fins->op1 == fins->op2) /* x o x ==> x */ + return LEFTFOLD; + return NEXTFOLD; +} + LJFOLD(BXOR any any) LJFOLDF(comm_bxor) { diff --git a/src/lj_vmmath.c b/src/lj_vmmath.c index b231d3e8..08ccf467 100644 --- a/src/lj_vmmath.c +++ b/src/lj_vmmath.c @@ -50,8 +50,8 @@ double lj_vm_foldarith(double x, double y, int op) #if LJ_HASJIT case IR_ATAN2 - IR_ADD: return atan2(x, y); break; case IR_LDEXP - IR_ADD: return ldexp(x, (int)y); break; - case IR_MIN - IR_ADD: return x > y ? y : x; break; - case IR_MAX - IR_ADD: return x < y ? y : x; break; + case IR_MIN - IR_ADD: return x < y ? x : y; break; + case IR_MAX - IR_ADD: return x > y ? x : y; break; #endif default: return x; } diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc index 21f7fecb..6b511347 100644 --- a/src/vm_arm.dasc +++ b/src/vm_arm.dasc @@ -1718,8 +1718,8 @@ static void build_subroutines(BuildCtx *ctx) |.endif |.endmacro | - | math_minmax math_min, gt, hi - | math_minmax math_max, lt, lo + | math_minmax math_min, gt, pl + | math_minmax math_max, lt, le | |//-- String library ----------------------------------------------------- | diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc index 313cc94f..80e795ae 100644 --- a/src/vm_arm64.dasc +++ b/src/vm_arm64.dasc @@ -1491,8 +1491,8 @@ static void build_subroutines(BuildCtx *ctx) | b <6 |.endmacro | - | math_minmax math_min, gt, hi - | math_minmax math_max, lt, lo + | math_minmax math_min, gt, pl + | math_minmax math_max, lt, le | |//-- String library ----------------------------------------------------- | diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc index 974047d3..cbf5fb9b 100644 --- a/src/vm_x64.dasc +++ b/src/vm_x64.dasc @@ -1874,7 +1874,7 @@ static void build_subroutines(BuildCtx *ctx) | jmp ->fff_res | |.macro math_minmax, name, cmovop, sseop - | .ffunc name + | .ffunc_1 name | mov RAd, 2 |.if DUALNUM | mov RB, [BASE] diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc index ab8e6f27..b839eff0 100644 --- a/src/vm_x86.dasc +++ b/src/vm_x86.dasc @@ -2292,7 +2292,7 @@ static void build_subroutines(BuildCtx *ctx) | xorps xmm4, xmm4; jmp <1 // Return +-Inf and +-0. | |.macro math_minmax, name, cmovop, sseop - | .ffunc name + | .ffunc_1 name | mov RA, 2 | cmp dword [BASE+4], LJ_TISNUM |.if DUALNUM diff --git a/test/tarantool-tests/gh-6163-jit-min-max.test.lua b/test/tarantool-tests/gh-6163-jit-min-max.test.lua new file mode 100644 index 00000000..d6eb3f3b --- /dev/null +++ b/test/tarantool-tests/gh-6163-jit-min-max.test.lua @@ -0,0 +1,20 @@ +local tap = require('tap') +jit.off() +jit.flush() + +local test = tap.test("gh-6163-jit-min-max") +test:plan(2) +-- +-- gh-6163: math.min/math.max success with no args +-- +local pcall = pcall + +jit.opt.start(0, 'hotloop=1') +jit.on() + +local r, _ = pcall(function() math.min() end) +test:ok(false == r) +r, _ = pcall(function() math.min() end) +test:ok(false == r) + +os.exit(test:check() and 0 or 1) -- 2.35.1