From 011c1afbde3a82712a4bffee1acc3bbdde319e2d Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 23 Oct 2023 08:00:48 -0700 Subject: [PATCH] Implement bit32.byteswap (#1075) I've decided to take a stab at implementing `bit32.byteswap` from the [recently merged RFC](https://github.com/Roblox/luau/blob/master/rfcs/function-bit32-byteswap.md). I asked on Discord for some guidance, but for the sake of posterity: this is my first time doing this and I am likely to have made some mistakes. The biggest gaps in this implementation are the lack of tests and the lack of native codegen support. I'd appreciate help with those since I'm not sure what's relevant for me to touch for tests, and I'm told that relevant assembler instructions don't exist publicly yet. Intuition tells me that Luau-side tests would go into `tests/conformance/bitwise.luau` but this is not well documented and I'm not sure how I'm meant to test built-in implementations. The current implementation compiles down to `bswap` and `rev` on x86 and ARM respectively when optimized. --------- Co-authored-by: Arseny Kapoulkine --- Analysis/src/EmbeddedBuiltinDefinitions.cpp | 1 + CodeGen/src/OptimizeConstProp.cpp | 1 + Common/include/Luau/Bytecode.h | 3 +++ Compiler/src/Builtins.cpp | 7 +++++++ VM/src/lbitlib.cpp | 15 +++++++++++++++ VM/src/lbuiltins.cpp | 19 +++++++++++++++++++ tests/Conformance.test.cpp | 1 + tests/conformance/bitwise.lua | 6 ++++++ 8 files changed, 53 insertions(+) diff --git a/Analysis/src/EmbeddedBuiltinDefinitions.cpp b/Analysis/src/EmbeddedBuiltinDefinitions.cpp index 65b04d62..dd2a5d19 100644 --- a/Analysis/src/EmbeddedBuiltinDefinitions.cpp +++ b/Analysis/src/EmbeddedBuiltinDefinitions.cpp @@ -21,6 +21,7 @@ declare bit32: { replace: (n: number, v: number, field: number, width: number?) -> number, countlz: (n: number) -> number, countrz: (n: number) -> number, + byteswap: (n: number) -> number, } declare math: { diff --git a/CodeGen/src/OptimizeConstProp.cpp b/CodeGen/src/OptimizeConstProp.cpp index c9b2dae6..6dfdbeb2 100644 --- a/CodeGen/src/OptimizeConstProp.cpp +++ b/CodeGen/src/OptimizeConstProp.cpp @@ -505,6 +505,7 @@ static void handleBuiltinEffects(ConstPropState& state, LuauBuiltinFunction bfid case LBF_GETMETATABLE: case LBF_TONUMBER: case LBF_TOSTRING: + case LBF_BIT32_BYTESWAP: break; case LBF_TABLE_INSERT: state.invalidateHeap(); diff --git a/Common/include/Luau/Bytecode.h b/Common/include/Luau/Bytecode.h index a4f1d67e..05916c2c 100644 --- a/Common/include/Luau/Bytecode.h +++ b/Common/include/Luau/Bytecode.h @@ -560,6 +560,9 @@ enum LuauBuiltinFunction // tonumber/tostring LBF_TONUMBER, LBF_TOSTRING, + + // bit32.byteswap(n) + LBF_BIT32_BYTESWAP, }; // Capture type, used in LOP_CAPTURE diff --git a/Compiler/src/Builtins.cpp b/Compiler/src/Builtins.cpp index a15c8f08..12b45290 100644 --- a/Compiler/src/Builtins.cpp +++ b/Compiler/src/Builtins.cpp @@ -4,6 +4,8 @@ #include "Luau/Bytecode.h" #include "Luau/Compiler.h" +LUAU_FASTFLAGVARIABLE(LuauBit32ByteswapBuiltin, false) + namespace Luau { namespace Compile @@ -166,6 +168,8 @@ static int getBuiltinFunctionId(const Builtin& builtin, const CompileOptions& op return LBF_BIT32_COUNTLZ; if (builtin.method == "countrz") return LBF_BIT32_COUNTRZ; + if (FFlag::LuauBit32ByteswapBuiltin && builtin.method == "byteswap") + return LBF_BIT32_BYTESWAP; } if (builtin.object == "string") @@ -402,6 +406,9 @@ BuiltinInfo getBuiltinInfo(int bfid) case LBF_TOSTRING: return {1, 1}; + + case LBF_BIT32_BYTESWAP: + return {1, 1, BuiltinInfo::Flag_NoneSafe}; }; LUAU_UNREACHABLE(); diff --git a/VM/src/lbitlib.cpp b/VM/src/lbitlib.cpp index 47445b80..627d599e 100644 --- a/VM/src/lbitlib.cpp +++ b/VM/src/lbitlib.cpp @@ -5,6 +5,8 @@ #include "lcommon.h" #include "lnumutils.h" +LUAU_FASTFLAGVARIABLE(LuauBit32Byteswap, false) + #define ALLONES ~0u #define NBITS int(8 * sizeof(unsigned)) @@ -210,6 +212,18 @@ static int b_countrz(lua_State* L) return 1; } +static int b_swap(lua_State* L) +{ + if (!FFlag::LuauBit32Byteswap) + luaL_error(L, "bit32.byteswap isn't enabled"); + + b_uint n = luaL_checkunsigned(L, 1); + n = (n << 24) | ((n << 8) & 0xff0000) | ((n >> 8) & 0xff00) | (n >> 24); + + lua_pushunsigned(L, n); + return 1; +} + static const luaL_Reg bitlib[] = { {"arshift", b_arshift}, {"band", b_and}, @@ -225,6 +239,7 @@ static const luaL_Reg bitlib[] = { {"rshift", b_rshift}, {"countlz", b_countlz}, {"countrz", b_countrz}, + {"byteswap", b_swap}, {NULL, NULL}, }; diff --git a/VM/src/lbuiltins.cpp b/VM/src/lbuiltins.cpp index a916f73a..bd4c28bc 100644 --- a/VM/src/lbuiltins.cpp +++ b/VM/src/lbuiltins.cpp @@ -1319,6 +1319,23 @@ static int luauF_tostring(lua_State* L, StkId res, TValue* arg0, int nresults, S return -1; } +static int luauF_byteswap(lua_State* L, StkId res, TValue* arg0, int nresults, StkId args, int nparams) +{ + if (nparams >= 1 && nresults <= 1 && ttisnumber(arg0)) + { + double a1 = nvalue(arg0); + unsigned n; + luai_num2unsigned(n, a1); + + n = (n << 24) | ((n << 8) & 0xff0000) | ((n >> 8) & 0xff00) | (n >> 24); + + setnvalue(res, double(n)); + return 1; + } + + return -1; +} + static int luauF_missing(lua_State* L, StkId res, TValue* arg0, int nresults, StkId args, int nparams) { return -1; @@ -1486,6 +1503,8 @@ const luau_FastFunction luauF_table[256] = { luauF_tonumber, luauF_tostring, + luauF_byteswap, + // When adding builtins, add them above this line; what follows is 64 "dummy" entries with luauF_missing fallback. // This is important so that older versions of the runtime that don't support newer builtins automatically fall back via luauF_missing. // Given the builtin addition velocity this should always provide a larger compatibility window than bytecode versions suggest. diff --git a/tests/Conformance.test.cpp b/tests/Conformance.test.cpp index 61ea2416..0ef32d1e 100644 --- a/tests/Conformance.test.cpp +++ b/tests/Conformance.test.cpp @@ -408,6 +408,7 @@ TEST_CASE("GC") TEST_CASE("Bitwise") { + ScopedFastFlag sffs{"LuauBit32Byteswap", true}; runConformance("bitwise.lua"); } diff --git a/tests/conformance/bitwise.lua b/tests/conformance/bitwise.lua index 3b117892..3c432869 100644 --- a/tests/conformance/bitwise.lua +++ b/tests/conformance/bitwise.lua @@ -134,6 +134,11 @@ assert(bit32.countrz(0x80000000) == 31) assert(bit32.countrz(0x40000000) == 30) assert(bit32.countrz(0x7fffffff) == 0) +-- testing byteswap +assert(bit32.byteswap(0x10203040) == 0x40302010) +assert(bit32.byteswap(0) == 0) +assert(bit32.byteswap(-1) == 0xffffffff) + --[[ This test verifies a fix in luauF_replace() where if the 4th parameter was not a number, but the first three are numbers, it will @@ -164,5 +169,6 @@ assert(bit32.btest("1", 3) == true) assert(bit32.countlz("42") == 26) assert(bit32.countrz("42") == 1) assert(bit32.extract("42", 1, 3) == 5) +assert(bit32.byteswap("0xa1b2c3d4") == 0xd4c3b2a1) return('OK')