Re: [Sbcl-commits] master: x86-64: don't combine arith + test.

Thanks for these fixes.  Did you find them from miscompiled examples or
from Thinking Very Hard?

Christophe

stassats via Sbcl-commits <[email protected]> writes:

> The branch "master" has been updated in SBCL:
>        via  6d2ffcff0b344fd9e2f25def3bede4e55e879cdd (commit)
>       from  6d747e69701c9b57e866bcf0ad94f492260f1ecc (commit)
>
> - Log -----------------------------------------------------------------
> commit 6d2ffcff0b344fd9e2f25def3bede4e55e879cdd
> Author: Stas Boukarev <[email protected]>
> Date:   Sat Nov 29 00:03:29 2025 +0300
>
>     x86-64: don't combine arith + test.
>     
>     It actually doesn't work because there can be multiple flag-reading
>     instructions.
> ---
>  src/compiler/x86-64/insts.lisp | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/src/compiler/x86-64/insts.lisp b/src/compiler/x86-64/insts.lisp
> index 4ee94720e..de73bf97b 100644
> --- a/src/compiler/x86-64/insts.lisp
> +++ b/src/compiler/x86-64/insts.lisp
> @@ -3574,6 +3574,7 @@
>  ;;; Per the processor manual, TEST clears OF and CF, so presumably
>  ;;; there is not a branch-if on either of those flags.
>  ;;; It shouldn't be a problem that removal of TEST leaves more flags affected.
> +#+nil ;; it doesn't account for multiple flag-reading instructions
>  (defpattern "ALU + test" ((add adc sub sbb and or xor neg sar shl shr) (test)) (stmt next)
>    (binding* (((size1 dst1 src1) (parse-2-operands stmt))
>               ((size2 dst2 src2) (parse-2-operands next))
> @@ -3614,16 +3615,14 @@
>                          :nl :nle :ng :nge)
>                         (and (eq size2 size1)
>                              ;; Assume there's no overflow for signed arithmetic
> -                            ;; Excluding
> -                            ;; sb-vm::fast--/fixnum=>fixnum
> -                            ;; sb-vm::fast--/signed=>signed
> -                            ;; sb-vm::fast---c/fixnum=>fixnum
> -                            ;; sb-vm::fast---c/signed=>signed
> -                            ;; because subtracting two negative numbers will set OF.
>                              (memq (vop-name (sb-assem::stmt-vop stmt))
> -                                  '(sb-vm::fast-+/fixnum=>fixnum
> +                                  '(sb-vm::fast--/fixnum=>fixnum
> +                                    sb-vm::fast-+/fixnum=>fixnum
> +                                    sb-vm::fast--/signed=>signed
>                                      sb-vm::fast-+/signed=>signed
> +                                    sb-vm::fast---c/fixnum=>fixnum
>                                      sb-vm::fast-+-c/fixnum=>fixnum
> +                                    sb-vm::fast---c/signed=>signed
>                                      sb-vm::fast-+-c/signed=>signed
>                                      sb-vm::fast-negate/fixnum
>                                      sb-vm::fast-negate/signed))))))))
>
> -----------------------------------------------------------------------
>
>
> hooks/post-receive