public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-5442] i386: Fix up <insn><dwi>3_doubleword_lowpart [PR112523]
@ 2023-11-14 12:20 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2023-11-14 12:20 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:aad65285a1c681feb9fc5b041c86d841b24c3d2a

commit r14-5442-gaad65285a1c681feb9fc5b041c86d841b24c3d2a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Nov 14 13:19:48 2023 +0100

    i386: Fix up <insn><dwi>3_doubleword_lowpart [PR112523]
    
    On Sun, Nov 12, 2023 at 09:03:42PM -0000, Roger Sayle wrote:
    > This patch improves register pressure during reload, inspired by PR 97756.
    > Normally, a double-word right-shift by a constant produces a double-word
    > result, the highpart of which is dead when followed by a truncation.
    > The dead code calculating the high part gets cleaned up post-reload, so
    > the issue isn't normally visible, except for the increased register
    > pressure during reload, sometimes leading to odd register assignments.
    > Providing a post-reload splitter, which clobbers a single wordmode
    > result register instead of a doubleword result register, helps (a bit).
    
    Unfortunately this broke bootstrap on i686-linux, broke all ACATS tests
    on x86_64-linux as well as miscompiled e.g. __floattisf in libgcc there
    as well.
    
    The bug is that shrd{l,q} instruction expects the low part of the input
    to be the same register as the output, rather than the high part as the
    patch implemented.
      split_double_mode (<DWI>mode, &operands[1], 1, &operands[1], &operands[3]);
    sets operands[1] to the lo_half and operands[3] to the hi_half, so if
    operands[0] is not the same register as operands[1] (rather than [3]) after
    RA, we should during splitting move operands[1] into operands[0].
    
    Your testcase:
    > #define MASK60 ((1ul << 60) - 1)
    > unsigned long foo (__uint128_t n)
    > {
    >   unsigned long a = n & MASK60;
    >   unsigned long b = (n >> 60);
    >   b = b & MASK60;
    >   unsigned long c = (n >> 120);
    >   return a+b+c;
    > }
    
    still has the same number of instructions.
    
    Bootstrapped/regtested on x86_64-linux (where it e.g. turns
                    === acats Summary ===
    -# of unexpected failures       2328
    +# of expected passes           2328
    +# of unexpected failures       0
    and fixes gcc.dg/torture/fp-int-convert-*timode.c FAILs as well)
    and i686-linux (where it previously didn't bootstrap, but compared to
    Friday evening's bootstrap the testresults are ok).
    
    2023-11-14  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/112523
            PR ada/112514
            * config/i386/i386.md (<insn><dwi>3_doubleword_lowpart): Move
            operands[1] aka low part of input rather than operands[3] aka high
            part of input to output if not the same register.

Diff:
---
 gcc/config/i386/i386.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 29289f48e9c..84cc50c7bb2 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -14825,8 +14825,8 @@
 {
   split_double_mode (<DWI>mode, &operands[1], 1, &operands[1], &operands[3]);
   operands[4] = GEN_INT ((<MODE_SIZE> * BITS_PER_UNIT) - INTVAL (operands[2]));
-  if (!rtx_equal_p (operands[0], operands[3]))
-    emit_move_insn (operands[0], operands[3]);
+  if (!rtx_equal_p (operands[0], operands[1]))
+    emit_move_insn (operands[0], operands[1]);
 })
 
 (define_insn "x86_64_shrd"

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-11-14 12:20 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-14 12:20 [gcc r14-5442] i386: Fix up <insn><dwi>3_doubleword_lowpart [PR112523] Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).