Admin message

Due to a large amount of spam we do not allow new users to create repositories, they are "external" users. If you are a new user and want to create a repository, for example for forking GHC, open a new issue on ghc/ghc using the "get-verified" issue template

AArch64 NCG: overflow bit incorrectly set for MO_S_Mul2
While reviewing !15619 I think I identified a bug in the AArch64 NCG in which the overflow bit for `MO_S_Mul2` is, I believe, flipped from what it should be in the sub-word-width case: ```hs | w < W64 , [src_a, src_b] <- arg_regs , [dst_needed, dst_hi, dst_lo] <- dest_regs -> do (reg_a', _format_x, code_a) <- getSomeReg src_a (reg_b', _format_y, code_b) <- getSomeReg src_b 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' 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. 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 (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. truncateReg w' w hi ``` Here, we do a logical shift right of `lo`, so we get `lo'`: * 1 if `lo` is negative * 0 otherwise This means that there is no overflow if `hi == -lo'`, as the comment writes. So we use `CMN`, and we interpret its result as follows: * EQ means `hi == -lo'` means no overflow * NE means `hi /= -lo'` means overflow So I believe the final condition is negated, and that it should be `CSET (OpReg w' nd) NE` instead of `CSET (OpReg w' nd) EQ`. The result is that the overflow bit is the opposite of what it is supposed to be for sub-word multiplication with overflow bit. I don't have a reproducer that triggers this bug at this time.
issue