Re: i386.md splits (hope final version)

[Jeffrey A Law <law at cygnus dot com>]

First, when discussing patches, please cc egcs-patches.  We want everyone
with an interest to be able to see the discussion and comment.

  > > OK, it sounds like the problem was that we did not do the extension in
  > > the reg that died, but instead in a new register.  That should be fixed
  > > in the register allocation pasess, not by allowing memory destinations
  > > for sign extension.
  > Thats one problem.
I think that is by far the more important problem.  The {sign,zero} extension
and the truncation patterns require more care to ensure that their source
and destination are the same register.  Many ports will benefit if we can
use the same register for the input and output more often.

What makes this particularly tricky is pseudos are accessed with a single
mode.  So, we have to rely on good register allocation and reloading to
assign the input and output pseudos to the same hard register.

One might consider treating the extension/truncation patterns like copies
to the register allocator.  ie, if we find an extension from (reg X) to
(reg Y) we may want to set qty_phys_copy_sugg as if it was a normal copy.
Might be worth an experiment.

  > Second is that even with ideal register allocation you still
  > need second register for second half of DI value, so you need to emit
  > something like:
  > move half1 to half2
  > shift half2
  > and reload or someone else will add moves to memory.
[ It's actually easier if you show rtl-like data.  It removes some of
  the potential language barriers.  It should be given that anyone
  interested in these kinds of details be able to understand RTL. ]

If the destination is only allowed to be a reg, then we have only two
choices.  1. The source and destination match, 2. The source and destination
do not match.  Everything else falls under one of those two cases as far
as I can tell.

--

If the source and destination match, then I think we want the splitter to
generate:

(set (highreg) (lowreg))
(set (highreg) (ashiftrt (highreg) (const_int 31)))

--

If the source and destination do not match this is the desired output
from the splitter:

(set (lowdest)  (low))
(set (highdest) (low))
(set (highdest) (ashiftrt (highdest) (const_int 31)))

--

If we take your suggestion and relax the extension pattern to allow a memory
destination, then the most efficient general code is something like this:

(set (lowmem) (low))
(set (scratch) (low))
(set (scratch) (ashiftrt (scratch) (const_int 31)))
(set (highmem) (scratch))

--

If you happen to know that the source register dies, then you can generate:
(set (lowmem) (low))
(set (low) (ashiftrt (low) (const_int 31)))
(set (highmem) (low))


So, it seems to me you need a pattern which accepts a REG or MEM as a
destination.  It splits into one of the 4 cases mentioned above.  ie
sreg == dreg
(set (high) (low))
(set (high) (ashiftrt (high) (const_int 31)))

sreg != dreg
(set (lowdest)  (low))
(set (highdest) (low))
(set (highdest) (ashiftrt (highdest) (const_int 31)))

dest in MEM, sreg does not die:
(set (lowmem) (low))
(set (scratch) (low))
(set (scratch) (ashiftrt (scratch) (const_int 31)))
(set (highmem) (scratch))


dest in MEM, sreg dies:
(set (lowmem) (low))
(set (low) (ashiftrt (low) (const_int 31)))
(set (highmem) (low))



That covers all the cases where the source operand is a register and the dest
operand is a register or memory.

I do not think your splitters handle all 4 cases correctly.  I think you
need to handle all 4 to ensure that you get good code for the new cases
without regressing on old cases.

I also see that you are allowing a memory source.  I don't see how that can
win.  You should go through the same kind of analysis I did above to show
a case where it is going to win.  I really doubt you'll find a case that
is a win.  


Here's what I hacked up.  This won't work with the current egcs sources
since it depends on being able to tell when we're splitting after reload
vs after flow2.  But you get the idea.

If you could set flow2_completed after the flow2 pass completes in toplev.c,
install the x86 backend changes, then look at your original testcases again,
it would be greatly appreciated.


*** i386.md.SAVE	Mon Nov  2 22:26:44 1998
--- i386.md	Tue Nov  3 00:05:58 1998
***************
*** 2045,2071 ****
  ;;- sign extension instructions
  
  (define_insn "extendsidi2"
!   [(set (match_operand:DI 0 "register_operand" "=r")
! 	(sign_extend:DI (match_operand:SI 1 "register_operand" "0")))]
!   ""
!   "*
! {
!   if (REGNO (operands[0]) == 0)
!     {
!       /* This used to be cwtl, but that extends HI to SI somehow.  */
! #ifdef INTEL_SYNTAX
!       return \"cdq\";
! #else
!       return \"cltd\";
! #endif
!     }
  
!   operands[1] = gen_rtx_REG (SImode, REGNO (operands[0]) + 1);
!   output_asm_insn (AS2 (mov%L0,%0,%1), operands);
  
!   operands[0] = GEN_INT (31);
!   return AS2 (sar%L1,%0,%1);
! }")
  
  ;; Note that the i386 programmers' manual says that the opcodes
  ;; are named movsx..., but the assembler on Unix does not accept that.
--- 2045,2101 ----
  ;;- sign extension instructions
  
  (define_insn "extendsidi2"
!   [(set (match_operand:DI 0 "nonimmediate_operand" "=A,?r,?r,?m")
! 	(sign_extend:DI (match_operand:SI 1 "register_operand" "0,0,r,r")))
!    (clobber (match_scratch:SI 2 "=X,X,X,r"))]
!   ""
!   "#")
! 
! ;; Extend to memory when source register dies case
! (define_split 
!   [(set (match_operand:DI 0 "memory_operand" "")
! 	(sign_extend:DI (match_operand:SI 1 "register_operand" "")))
!    (clobber (match_operand:SI 2 "register_operand" ""))]
!   "flow2_completed && dead_or_set_p (insn, operands[1])"
!   [(set (match_dup 3) (match_dup 1))
!    (set (match_dup 1)
!         (ashiftrt:SI (match_dup 1) (const_int 31)))
!    (set (match_dup 4) (match_dup 1))]
!   "split_di (&operands[0], 1, &operands[3], &operands[4]);")
! 
! 
! ;; Extend to memory when source register does not die case
! (define_split 
!   [(set (match_operand:DI 0 "memory_operand" "")
! 	(sign_extend:DI (match_operand:SI 1 "register_operand" "")))
!    (clobber (match_operand:SI 2 "register_operand" ""))]
!   "flow2_completed"
!   [(set (match_dup 3) (match_dup 1))
!    (set (match_dup 2) (match_dup 1))
!    (set (match_dup 2) (ashiftrt:SI (match_dup 2) (const_int 31)))
!    (set (match_dup 4) (match_dup 2))]
!   "split_di (&operands[0], 1, &operands[3], &operands[4]);")
  
! ;; Extend to register when source and destination registers match.
! (define_split 
!   [(set (match_operand:DI 0 "register_operand" "")
! 	(sign_extend:DI (match_operand:SI 1 "register_operand" "")))
!    (clobber (scratch:SI))]
!   "reload_completed && true_regnum (operands[0]) == true_regnum (operands[1])"
!   [(set (match_dup 4) (match_dup 1))
!    (set (match_dup 4) (ashiftrt:SI (match_dup 4) (const_int 31)))]
!   "split_di (&operands[0], 1, &operands[3], &operands[4]);")
  
! ;; Extend to register when source and destination registers do not match
! (define_split 
!   [(set (match_operand:DI 0 "register_operand" "")
! 	(sign_extend:DI (match_operand:SI 1 "register_operand" "")))
!    (clobber (scratch:SI))]
!   "flow2_completed"
!   [(set (match_dup 3) (match_dup 1))
!    (set (match_dup 4) (match_dup 1))
!    (set (match_dup 4) (ashiftrt:SI (match_dup 4) (const_int 31)))]
!   "split_di (&operands[0], 1, &operands[3], &operands[4]);")
  
  ;; Note that the i386 programmers' manual says that the opcodes
  ;; are named movsx..., but the assembler on Unix does not accept that.

Jeff