ix86_expand_setcc to clear + set strict_low_part

[Alexandre Oliva <oliva at lsd dot ic dot unicamp dot br>]

I was investigating why ix86_expand_setcc said:

  type = 0;
  /* %%% reload problems with in-out.  Revisit.  */
  type = 1;

In fact, removing the assignment `type=1' caused stage1 to fail to
build stage2, with various ICEs within verify_local_live_at_start(),
at the end of peephole2_optimize().

I managed to create a minimal testcase:

inline int
foo (int **q) {
  return *q && **q;
}

void
bar () {
  int **p;
  if (foo (p))
    abort ();
}


The problem was that, in the following basic block:

(note 49 41 42 [bb 1] NOTE_INSN_BASIC_BLOCK)

(insn:QI 42 49 43 (set (reg:CCNO 17 flags)
        (compare:CCNO (mem:SI (reg:SI 1 edx) 3)
            (const_int 0 [0x0]))) 4 {cmpsi_0} (nil)
    (expr_list:REG_DEAD (reg:SI 1 edx)
        (nil)))

(insn 43 42 21 (set (strict_low_part (reg:QI 0 al))
        (ne:QI (reg:CCNO 17 flags)
            (const_int 0 [0x0]))) 420 {*setcc_2} (insn_list 42 (insn_list 41 (insn_list 42 (nil))))
    (expr_list:REG_DEAD (reg:CCNO 17 flags)
        (nil)))

it would apply:

;; Don't compare memory with zero, load and use a test instead.
(define_peephole2
  [(set (reg:CCNO 17)
	(compare:CCNO (match_operand:SI 0 "memory_operand" "")
	(const_int 0)))
   (match_scratch:SI 3 "r")]
  "! optimize_size"
   [(set (match_dup 3) (match_dup 0))
    (set (reg:CCNO 17) (compare:CCNO (match_dup 3) (const_int 0)))]
  "")

However, for various reasons, it would choose `eax' as the scratch
register, but eax was live in the beginning of the basic block.
However, since `eax' was chosen for scratch, it was killed, and life
analysis within verify_local_live_at_start() noticed the change and
complained.


The first reason why `eax' was chosen was because find_basic_block(),
called from mark_target_live_regs(), called from find_free_register(),
would try to find the beginning of the basic block, iterating on the
insns backwards, but skipping notes, particularly, the
NOTE_INSN_BASIC_BLOCK it should be looking for.

Having fixed this problem, I happily re-compiled cc1, just to find
that the problem remained!

Further investigation revealed that find_dead_or_set_registers(), also
called from mark_target_live_regs(), would consider `eax' dead before
`(insn 43 42 21 (set (strict_low_part (reg:QI 0 al)) ...', even though
the higher-order bits of `eax' were still live.

After arranging for mark_referenced_resources(), called from
find_dead_or_set_registers(), to mark the whole of the strict_low_part
as used, I managed to compile the testcase, and to bootstrap gcc
without any regressions.

Here's the patch.  Ok to install?

Index: gcc/ChangeLog
from  Alexandre Oliva  <oliva@lsd.ic.unicamp.br>
	
	* resource.c (find_basic_block): Get basic-block number from
	NOTE_INSN_BASIC_BLOCKs.
	(mark_referenced_resources): Mark a set strict_low_part as used.
	* config/i386/i386.c (ix86_expand_setcc): Re-enable clear + set
	strict_low_part when possible.
	
Index: gcc/resource.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/resource.c,v
retrieving revision 1.21
diff -u -p -r1.21 resource.c
--- gcc/resource.c	1999/11/02 23:43:44	1.21
+++ gcc/resource.c	2000/01/10 09:34:21
@@ -125,14 +125,22 @@ find_basic_block (insn)
   /* Scan backwards to the previous BARRIER.  Then see if we can find a
      label that starts a basic block.  Return the basic block number.  */
 
-  for (insn = prev_nonnote_insn (insn);
-       insn && GET_CODE (insn) != BARRIER;
-       insn = prev_nonnote_insn (insn))
+  for (insn = PREV_INSN (insn);
+       insn && GET_CODE (insn) != BARRIER
+	 && ! (GET_CODE (insn) == NOTE
+	       && NOTE_LINE_NUMBER (insn) == NOTE_INSN_BASIC_BLOCK);
+       insn = PREV_INSN (insn))
     ;
 
   /* The start of the function is basic block zero.  */
   if (insn == 0)
     return 0;
+  else if (GET_CODE (insn) == NOTE
+	   /* implied from loop exit condition above:
+	      && NOTE_LINE_NUMBER (insn) == NOTE_INSN_BASIC_BLOCK
+	   */
+	   )
+    return NOTE_BASIC_BLOCK (insn)->index;
 
   /* See if any of the upcoming CODE_LABELs start a basic block.  If we reach
      anything other than a CODE_LABEL or note, we can't find this code.  */
@@ -275,7 +283,8 @@ mark_referenced_resources (x, res, inclu
 	mark_referenced_resources (x, res, 0);
       else if (GET_CODE (x) == SUBREG)
 	x = SUBREG_REG (x);
-      if (GET_CODE (x) == MEM)
+      if (GET_CODE (x) == MEM
+	  || GET_CODE (x) == STRICT_LOW_PART)
 	mark_referenced_resources (XEXP (x, 0), res, 0);
       return;
 
Index: gcc/config/i386/i386.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/i386/i386.c,v
retrieving revision 1.118
diff -u -p -r1.118 i386.c
--- gcc/config/i386/i386.c	2000/01/04 14:44:03	1.118
+++ gcc/config/i386/i386.c	2000/01/10 09:34:23
@@ -4340,8 +4340,6 @@ ix86_expand_setcc (code, unordered, dest
   */
 
   type = 0;
-  /* %%% reload problems with in-out.  Revisit.  */
-  type = 1;
 
   if (GET_MODE (dest) == QImode)
     type = 2;

-- 
Alexandre Oliva http://www.ic.unicamp.br/~oliva IC-Unicamp, Bra[sz]il
oliva@{lsd.ic.unicamp.br,guarana.{org,com}} aoliva@{acm,computer}.org
oliva@{gnu.org,kaffe.org,{egcs,sourceware}.cygnus.com,samba.org}
** I may forward mail about projects to mailing lists; please use them