Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC Commits: 636c1c7a by Ian Duncan at 2026-06-16T20:26:50-04:00 AArch64: use SXTH, not SXTW, for W32 signExtendReg signExtendReg was using SXTH (sign-extend halfword, 16-bit) for W32-to-W64 sign extension. This should be SXTW (sign-extend word, 32-bit). SXTH only sign-extends the lower 16 bits, producing wrong results for 32-bit values whose bit 15 differs from bit 31. Other fixes: - At sub-W64, code gen for MO_S_Mul2 should use W32 registers for SMULL source operands as per the ARM spec (SMULL Xd, Wn, Wm), and not W64. - Ensure signExtendReg uses the source width for the source operand in SXTW/SXTH/SXTB instructions. GNU as requires sxtw Xd,Wn (not sxtw Xd,Xn), while LLVM's integrated assembler on macOS is lenient. - Fix overflow flag computation for `MO_S_Mul2`. The overflow bit was exactly inverted for sub-W64 operands. Fixes #26978 and #27047 - - - - - 14 changed files: - + changelog.d/T26978 - + changelog.d/T27047 - compiler/GHC/CmmToAsm/AArch64/CodeGen.hs - compiler/GHC/CmmToAsm/AArch64/Instr.hs - compiler/GHC/CmmToAsm/AArch64/Ppr.hs - + testsuite/tests/codeGen/should_gen_asm/aarch64-sxth-mul2.asm - + testsuite/tests/codeGen/should_gen_asm/aarch64-sxth-mul2.cmm - + testsuite/tests/codeGen/should_gen_asm/aarch64-sxtw.asm - + testsuite/tests/codeGen/should_gen_asm/aarch64-sxtw.cmm - testsuite/tests/codeGen/should_gen_asm/all.T - + testsuite/tests/codeGen/should_run/aarch64-sxtw-cmm.cmm - + testsuite/tests/codeGen/should_run/aarch64-sxtw-run.hs - + testsuite/tests/codeGen/should_run/aarch64-sxtw-run.stdout - testsuite/tests/codeGen/should_run/all.T Changes: ===================================== changelog.d/T26978 ===================================== @@ -0,0 +1,14 @@ +section: compiler +issues: #26978 +mrs: !15619 +synopsis: + AArch64 code generation: use ``SXTH`` instead of ``SXTW`` for ``W32`` sign extension +description: + Fix the AArch64 code generator incorrectly using using ``SXTH`` + (sign-extend halfword, 16-bit) for ``W32``-to-``W64`` sign extension, and + always use the source width for the source operand in sign extension + instructions. + + Also ensures that sub-W64 code generation for ``MO_S_Mul2`` uses ``W32`` + registers as mandated by the ARM specification. + ===================================== changelog.d/T27047 ===================================== @@ -0,0 +1,9 @@ +section: compiler +issues: #27047 +mrs: !15619 +synopsis: + Fix incorrect overflow bit for ``MUL2`` on AArch64 for sub-``W64`` operands. +description: + This fixes the computation of the overflow bit for ``MO_S_Mul2`` on + sub-``W64`` operands; the old condition was exactly inverted. + ===================================== compiler/GHC/CmmToAsm/AArch64/CodeGen.hs ===================================== @@ -1894,15 +1894,15 @@ signExtendReg w w' r = W64 -> noop W32 | w' == W32 -> noop - | otherwise -> extend SXTH + | otherwise -> extend SXTW W16 -> extend SXTH W8 -> extend SXTB _ -> panic "intOp" where noop = return (r, nilOL) extend instr = do - r' <- getNewRegNat II64 - return (r', unitOL $ instr (OpReg w' r') (OpReg w' r)) + r' <- getNewRegNat (intFormat w') + return (r', unitOL $ instr (OpReg w' r') (OpReg w r)) -- | Instructions to truncate the value in the given register from width @w@ -- down to width @w'@. @@ -2333,41 +2333,35 @@ genCCall target dest_regs arg_regs = do let lo = getRegisterReg platform (CmmLocal dst_lo) hi = getRegisterReg platform (CmmLocal dst_hi) nd = getRegisterReg platform (CmmLocal dst_needed) - -- Do everything in a full 64 bit registers w' = platformWordWidth platform - (reg_a, code_a') <- signExtendReg w w' reg_a' - (reg_b, code_b') <- signExtendReg w w' reg_b' + -- Sign-extend inputs to W32 for SMULL (Xd = Wn * Wm). + -- signExtendReg always allocates a fresh temp for w < W32, + -- and is a noop for W32 (safe: SMULL reads both sources + -- atomically before writing the destination). + (reg_a, code_a') <- signExtendReg w W32 reg_a' + (reg_b, code_b') <- signExtendReg w W32 reg_b' return $ code_a `appOL` code_b `appOL` code_a' `appOL` code_b' `snocOL` - -- the low 2w' of lo contains the full multiplication; - -- eg: int8 * int8 -> int16 result - -- so lo is in the last w of the register, and hi is in the second w. - SMULL (OpReg w' lo) (OpReg w' reg_a) (OpReg w' reg_b) `snocOL` - -- Make sure we hold onto the sign bits for dst_needed - ASR (OpReg w' hi) (OpReg w' lo) (OpImm (ImmInt $ widthInBits w)) `appOL` - -- lo can now be truncated so we can get at it's top bit easily. + -- SMULL Xd, Wn, Wm: multiply two W32 values producing a + -- 64-bit result. The low w bits of lo contain the truncated + -- product, and hi gets the overflow (sign extension bits). + SMULL (OpReg w' lo) (OpReg W32 reg_a) (OpReg W32 reg_b) `snocOL` + ASR (OpReg w' hi) (OpReg w' lo) (OpImm (ImmInt $ widthInBits w)) `appOL` truncateReg w' w lo `snocOL` - -- Note the use of CMN (compare negative), not CMP: we want to - -- test if the top half is negative one and the top - -- bit of the bottom half is positive one. eg: - -- hi = 0b1111_1111 (actually 64 bits) - -- lo = 0b1010_1111 (-81, so the result didn't need the top half) - -- lo' = ASR(lo,7) (second reg of SMN) - -- = 0b0000_0001 (theeshift gives us 1 for negative, - -- and 0 for positive) - -- hi == -lo'? - -- 0b1111_1111 == 0b1111_1111 (yes, top half is just overflow) - -- Another way to think of this is if hi + lo' == 0, which is what - -- CMN really is under the hood. + -- CMN (compare negative) tests hi + lo' == 0, i.e. hi == -lo'. + -- lo' = LSR(lo, w-1) gives 1 if lo is negative, 0 if positive. + -- No overflow iff hi is the sign extension of lo: + -- lo positive (bit w-1 = 0) => lo' = 0, need hi == 0 + -- lo negative (bit w-1 = 1) => lo' = 1, need hi == -1 + -- CMN sets Z when hi + lo' == 0 (no overflow), so we use + -- NE to set nd = 1 when overflow occurred. CMN (OpReg w' hi) (OpRegShift w' lo SLSR (widthInBits w - 1)) `snocOL` - -- Set dst_needed to 1 if hi and lo' were (negatively) equal - CSET (OpReg w' nd) EQ `appOL` - -- Finally truncate hi to drop any extraneous sign bits. + CSET (OpReg w' nd) NE `appOL` truncateReg w' w hi -- Can't handle > 64 bit operands | otherwise -> unsupported (MO_S_Mul2 w) ===================================== compiler/GHC/CmmToAsm/AArch64/Instr.hs ===================================== @@ -105,6 +105,7 @@ regUsageOfInstr platform instr = case instr of SXTB dst src -> usage (regOp src, regOp dst) UXTB dst src -> usage (regOp src, regOp dst) SXTH dst src -> usage (regOp src, regOp dst) + SXTW dst src -> usage (regOp src, regOp dst) UXTH dst src -> usage (regOp src, regOp dst) CLZ dst src -> usage (regOp src, regOp dst) RBIT dst src -> usage (regOp src, regOp dst) @@ -296,6 +297,7 @@ patchRegsOfInstr instr env = case instr of SXTB o1 o2 -> SXTB (patchOp o1) (patchOp o2) UXTB o1 o2 -> UXTB (patchOp o1) (patchOp o2) SXTH o1 o2 -> SXTH (patchOp o1) (patchOp o2) + SXTW o1 o2 -> SXTW (patchOp o1) (patchOp o2) UXTH o1 o2 -> UXTH (patchOp o1) (patchOp o2) CLZ o1 o2 -> CLZ (patchOp o1) (patchOp o2) RBIT o1 o2 -> RBIT (patchOp o1) (patchOp o2) @@ -699,8 +701,7 @@ data Instr | UXTB Operand Operand | SXTH Operand Operand | UXTH Operand Operand - -- | SXTW Operand Operand - -- | SXTX Operand Operand + | SXTW Operand Operand | PUSH_STACK_FRAME | POP_STACK_FRAME -- 1. Arithmetic Instructions ---------------------------------------------- @@ -857,6 +858,7 @@ instrCon i = SXTB{} -> "SXTB" UXTB{} -> "UXTB" SXTH{} -> "SXTH" + SXTW{} -> "SXTW" UXTH{} -> "UXTH" PUSH_STACK_FRAME{} -> "PUSH_STACK_FRAME" POP_STACK_FRAME{} -> "POP_STACK_FRAME" ===================================== compiler/GHC/CmmToAsm/AArch64/Ppr.hs ===================================== @@ -516,6 +516,7 @@ pprInstr platform instr = case instr of SXTB o1 o2 -> op2 (text "\tsxtb") o1 o2 UXTB o1 o2 -> op2 (text "\tuxtb") o1 o2 SXTH o1 o2 -> op2 (text "\tsxth") o1 o2 + SXTW o1 o2 -> op2 (text "\tsxtw") o1 o2 UXTH o1 o2 -> op2 (text "\tuxth") o1 o2 -- 3. Logical and Move Instructions ------------------------------------------ ===================================== testsuite/tests/codeGen/should_gen_asm/aarch64-sxth-mul2.asm ===================================== @@ -0,0 +1 @@ +sxth ===================================== testsuite/tests/codeGen/should_gen_asm/aarch64-sxth-mul2.cmm ===================================== @@ -0,0 +1,12 @@ +#include "Cmm.h" + +// Exercises MO_S_Mul2 W16: signExtendReg W16 W32 must emit SXTH to +// sign-extend the 16-bit inputs to 32-bit before SMULL. +testMul2W16 (W_ buffer) { + I16 a, b, needed, hi, lo; + a = I16[buffer]; + b = I16[buffer + 2]; + (needed, hi, lo) = prim %mul2_16(a, b); + I16[buffer + 4] = lo; + return(); +} ===================================== testsuite/tests/codeGen/should_gen_asm/aarch64-sxtw.asm ===================================== @@ -0,0 +1 @@ +smull ===================================== testsuite/tests/codeGen/should_gen_asm/aarch64-sxtw.cmm ===================================== @@ -0,0 +1,13 @@ +#include "Cmm.h" + +// Exercises MO_S_Mul2 W32, which uses SMULL (Xd = Wn * Wm). +// After the fix, signExtendReg W32 W32 is a noop (no SXTW needed), +// but the SMULL instruction itself must still appear. +testMul2W32 (W_ buffer) { + I32 a, b, needed, hi, lo; + a = I32[buffer]; + b = I32[buffer + 4]; + (needed, hi, lo) = prim %mul2_32(a, b); + I32[buffer + 8] = lo; + return(); +} ===================================== testsuite/tests/codeGen/should_gen_asm/all.T ===================================== @@ -31,3 +31,5 @@ is_aarch64_codegen = [ # AArch64-specific tests test('aarch64-ushr-subword', is_aarch64_codegen, compile_grep_asm, ['hs', True, '-O']) test('aarch64-shl-subword', is_aarch64_codegen, compile_grep_asm, ['hs', True, '-O']) +test('aarch64-sxtw', is_aarch64_codegen, compile_grep_asm, ['cmm', True, '']) +test('aarch64-sxth-mul2', is_aarch64_codegen, compile_grep_asm, ['cmm', True, '']) ===================================== testsuite/tests/codeGen/should_run/aarch64-sxtw-cmm.cmm ===================================== @@ -0,0 +1,41 @@ +#include "Cmm.h" + +// MO_S_Mul2 W32: signExtendReg W32 W32 is a noop (inputs already W32). +// Tests the SMULL path with values whose bit 15 != bit 31 (the old SXTH +// bug would corrupt these). +runMul2W32zh (W_ x) { + I32 a, b, needed, hi, lo; + a = %lobits32(x); + b = 2::I32; + (needed, hi, lo) = prim %mul2_32(a, b); + return(TO_W_(lo)); +} + +// MO_S_Mul2 W16: signExtendReg W16 W32 emits SXTH before SMULL. +runMul2W16zh (W_ x) { + I16 a, b, needed, hi, lo; + a = %lobits16(x); + b = 2::I16; + (needed, hi, lo) = prim %mul2_16(a, b); + return(TO_W_(lo)); +} + +// MO_S_Mul2 W8: signExtendReg W8 W32 emits SXTB before SMULL. +runMul2W8zh (W_ x) { + I8 a, b, needed, hi, lo; + a = %lobits8(x); + b = 2::I8; + (needed, hi, lo) = prim %mul2_8(a, b); + return(TO_W_(lo)); +} + +// MO_S_Mul2 W16 returning (needed, hi, lo) as a packed triple. +// Tests the CSET NE overflow detection: needed=1 when overflow occurs. +// Returns (needed << 32) | (lo & 0xFFFF) so the Haskell side can inspect both. +runMul2W16Overflowzh (W_ x, W_ y) { + I16 a, b, needed, hi, lo; + a = %lobits16(x); + b = %lobits16(y); + (needed, hi, lo) = prim %mul2_16(a, b); + return(TO_W_(needed) * 0x100000000 + (TO_W_(lo) `and` 0xFFFF)); +} ===================================== testsuite/tests/codeGen/should_run/aarch64-sxtw-run.hs ===================================== @@ -0,0 +1,60 @@ +{-# LANGUAGE ForeignFunctionInterface #-} +{-# LANGUAGE GHCForeignImportPrim #-} +{-# LANGUAGE MagicHash #-} +{-# LANGUAGE UnliftedFFITypes #-} +module Main where + +import GHC.Exts +import Data.Bits (shiftR, (.&.)) + +foreign import prim "runMul2W32zh" runMul2W32# :: Word# -> Word# +foreign import prim "runMul2W16zh" runMul2W16# :: Word# -> Word# +foreign import prim "runMul2W8zh" runMul2W8# :: Word# -> Word# +foreign import prim "runMul2W16Overflowzh" + runMul2W16Overflow# :: Word# -> Word# -> Word# + +mul2W32 :: Word -> Word +mul2W32 (W# x) = W# (runMul2W32# x) + +mul2W16 :: Word -> Word +mul2W16 (W# x) = W# (runMul2W16# x) + +mul2W8 :: Word -> Word +mul2W8 (W# x) = W# (runMul2W8# x) + +mul2W16Overflow :: Word -> Word -> (Word, Word) +mul2W16Overflow (W# x) (W# y) = + let r = W# (runMul2W16Overflow# x y) + needed = r `shiftR` 32 + lo = r .&. 0xFFFF + in (needed, lo) + +main :: IO () +main = do + -- W32: 50000 has bit 15 set, bit 31 clear. The old SXTH bug would + -- sign-extend from bit 15, corrupting the value before SMULL. + putStrLn $ "W32 50000*2 = " ++ show (mul2W32 50000) + putStrLn $ "W32 1*2 = " ++ show (mul2W32 1) + putStrLn $ "W32 70000*2 = " ++ show (mul2W32 70000) + + -- W16: exercises signExtendReg W16 W32 (SXTH before SMULL). + putStrLn $ "W16 200*2 = " ++ show (mul2W16 200) + putStrLn $ "W16 100*2 = " ++ show (mul2W16 100) + + -- W8: exercises signExtendReg W8 W32 (SXTB before SMULL). + putStrLn $ "W8 50*2 = " ++ show (mul2W8 50) + putStrLn $ "W8 3*2 = " ++ show (mul2W8 3) + + -- Overflow detection (CSET NE fix): + -- 200 * 200 = 40000, overflows I16 [-32768, 32767]. needed=1. + let (n1, lo1) = mul2W16Overflow 200 200 + putStrLn $ "W16 200*200: needed=" ++ show n1 ++ " lo=" ++ show lo1 + -- 100 * 2 = 200, fits in I16. needed=0. + let (n2, lo2) = mul2W16Overflow 100 2 + putStrLn $ "W16 100*2: needed=" ++ show n2 ++ " lo=" ++ show lo2 + -- 181 * 181 = 32761, just under 32767. needed=0. + let (n3, lo3) = mul2W16Overflow 181 181 + putStrLn $ "W16 181*181: needed=" ++ show n3 ++ " lo=" ++ show lo3 + -- 182 * 182 = 33124, just over 32767. needed=1. + let (n4, lo4) = mul2W16Overflow 182 182 + putStrLn $ "W16 182*182: needed=" ++ show n4 ++ " lo=" ++ show lo4 ===================================== testsuite/tests/codeGen/should_run/aarch64-sxtw-run.stdout ===================================== @@ -0,0 +1,11 @@ +W32 50000*2 = 100000 +W32 1*2 = 2 +W32 70000*2 = 140000 +W16 200*2 = 400 +W16 100*2 = 200 +W8 50*2 = 100 +W8 3*2 = 6 +W16 200*200: needed=1 lo=40000 +W16 100*2: needed=0 lo=200 +W16 181*181: needed=0 lo=32761 +W16 182*182: needed=1 lo=33124 ===================================== testsuite/tests/codeGen/should_run/all.T ===================================== @@ -289,3 +289,9 @@ test('T27072w', [req_c, js_skip, when(opsys('darwin'), skip)], # AArch64-specific runtime tests test('aarch64-ushr-subword-run', [unless(arch('aarch64'), skip)], compile_and_run, ['-O']) test('aarch64-subword-ops', [unless(arch('aarch64'), skip)], compile_and_run, ['-O']) +test('aarch64-sxtw-run', + [ extra_files(['aarch64-sxtw-cmm.cmm']), + unless(arch('aarch64'), skip), + when(unregisterised(), skip)], + multi_compile_and_run, + ['aarch64-sxtw-run', [('aarch64-sxtw-cmm.cmm', '')], '-O']) View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/636c1c7ae47495f022affa501ea0a40c... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/636c1c7ae47495f022affa501ea0a40c... You're receiving this email because of your account on gitlab.haskell.org.