This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA:] Fix MMIX breakage: add missing validation ingcse.c:compute_ld_motion_mems


mmix-knuth-mmixware was broken a few days ago, as reported by Andreas
Jaeger in <URL:http://gcc.gnu.org/ml/gcc-bugs/2003-04/msg00154.html>:

/mmix/mmix/build/gcc/xgcc -B/mmix/mmix/build/gcc/ -nostdinc -B/mmix/mmix/build/mmix/gnuabi/newlib/ -isystem
/mmix/mmix/build/mmix/gnuabi/newlib/targ-include -isystem /mmix/cross-mmix-sources/combined/newlib/libc/include
-B/mmix/mmix/install/mmix/bin/ -B/mmix/mmix/install/mmix/lib/ -isystem /mmix/mmix/install/mmix/include -isystem
/mmix/mmix/install/mmix/sys-include -L/mmix/mmix/build/ld  -mabi=gnu -DPACKAGE=\"newlib\" -DVERSION=\"1.11.0\"  -I.
-I/mmix/cross-mmix-sources/combined/newlib/libm/math  -I/mmix/cross-mmix-sources/combined/newlib/libm/math/../common -O2
-DCOMPACT_CTYPE -fno-builtin    -O2 -g -O2  -O2 -g -O2  -mabi=gnu -c /mmix/cross-mmix-sources/combined/newlib/libm/math/ef_cosh.c
/mmix/cross-mmix-sources/combined/newlib/libm/math/ef_cosh.c: In function `__ieee754_coshf':
/mmix/cross-mmix-sources/combined/newlib/libm/math/ef_cosh.c:71: error: unrecognizable insn:
(insn 405 81 82 3 (nil) (set (reg:SF 478)
        (float_truncate:SF (reg:DF 306))) -1 (insn_list 79 (nil))
    (expr_list:REG_DEAD (reg:DF 306)
        (nil)))

The breakage is because this transformation is done without validation:

      /* We replace  SET mem = expr   with
	   SET reg = expr
	   SET mem = reg , where reg is the
	   reaching reg used in the load.  */

The comment is from gcse.c:update_ld_motion_stores.  The validation
that must be done, is to check that a MEM-dest *can* be replaced by a
REG-dest in that insn and to skip the "load motion" if not.  Recent
work in gcse.c has exposed this missing validation bug, which seems to
have been there from when update_ld_motion_stores was first added,
back in version 1.121.

For MMIX, truncdfsf2 insn takes only a memory destination; not a
register.  The define_expand for that name creates a stack temporary
if it's fed a register destination.  To wit, the insn above isn't
valid, but this one is:
 (insn 113 110 400 5 0x401ba280 (set (mem/f:SF (reg/f:DI 309) [2 S4 A32])
        (float_truncate:SF (reg:DF 323))) 43 {*truncdfsf2_real} (nil)
    (nil))

Now, the gcse code has code performing *some* validation, but it's not
done for a MEM destination as above.  The right place to do proper
MEM-dest validation seems to be the function that records *which*
MEM-dests should be considered for moving; there's filtering done
which excludes among others complicated(?) MEM constructs.  A nearby
function will do the actual check source-with-dest-as-reg validation
for us; want_to_gcse_p, except we must check for a REG source
ourselves.

Bootstrapped (no Ada) and checked with no regressions on
i686-pc-linux-gnu where it seems to fix
"g77.f-torture/execute/f90-intrinsic-bit.f execution, -O3
-fomit-frame-pointer -funroll-all-loops -finline-functions".  Built
and checked with no regressions on cross to cris-axis-linux-gnu,
h8300-hitachi-hms, m32r-unknown-elf, mn10300-unknown-elf and
arm-unknown-elf.

Ok to commit?

	* gcse.c (compute_ld_motion_mems): For MEM destinations, only
	consider those to be movable where the source matches
	want_to_gcse_p.
	(update_ld_motion_stores): In comment, refer to
	compute_ld_motion_mems for validity of replacement.

Index: gcse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gcse.c,v
retrieving revision 1.244
diff -p -c -r1.244 gcse.c
*** gcse.c	3 Apr 2003 19:20:03 -0000	1.244
--- gcse.c	7 Apr 2003 21:10:20 -0000
*************** invalidate_any_buried_refs (x)
*** 6771,6781 ****
      }
  }

! /* Find all the 'simple' MEMs which are used in LOADs and STORES. Simple
!    being defined as MEM loads and stores to symbols, with no
!    side effects and no registers in the expression. If there are any
!    uses/defs which don't match this criteria, it is invalidated and
!    trimmed out later.  */

  static void
  compute_ld_motion_mems ()
--- 6771,6783 ----
      }
  }

! /* Find all the 'simple' MEMs which are used in LOADs and STORES.  Simple
!    being defined as MEM loads and stores to symbols, with no side effects
!    and no registers in the expression.  For a MEM destination, we also
!    check that the insn is still valid if we replace the destination with a
!    REG, as is done in update_ld_motion_stores.  If there are any uses/defs
!    which don't match this criteria, they are invalidated and trimmed out
!    later.  */

  static void
  compute_ld_motion_mems ()
*************** compute_ld_motion_mems ()
*** 6823,6829 ****
  		      ptr = ldst_entry (dest);

  		      if (GET_CODE (src) != MEM
! 			  && GET_CODE (src) != ASM_OPERANDS)
  			ptr->stores = alloc_INSN_LIST (insn, ptr->stores);
  		      else
  			ptr->invalid = 1;
--- 6825,6834 ----
  		      ptr = ldst_entry (dest);

  		      if (GET_CODE (src) != MEM
! 			  && GET_CODE (src) != ASM_OPERANDS
! 			  /* Check for REG manually since want_to_gcse_p
! 			     returns 0 for all REGs.  */
! 			  && (REG_P (src) || want_to_gcse_p (src)))
  			ptr->stores = alloc_INSN_LIST (insn, ptr->stores);
  		      else
  			ptr->invalid = 1;
*************** update_ld_motion_stores (expr)
*** 6918,6927 ****
  	 matter to set the reaching reg everywhere...  some might be
  	 dead and should be eliminated later.  */

!       /* We replace  SET mem = expr   with
! 	   SET reg = expr
! 	   SET mem = reg , where reg is the
! 	   reaching reg used in the load.  */
        rtx list = mem_ptr->stores;

        for ( ; list != NULL_RTX; list = XEXP (list, 1))
--- 6923,6932 ----
  	 matter to set the reaching reg everywhere...  some might be
  	 dead and should be eliminated later.  */

!       /* We replace (set mem expr) with (set reg expr) (set mem reg)
! 	 where reg is the reaching reg used in the load.  We checked in
! 	 compute_ld_motion_mems that we can replace (set mem expr) with
! 	 (set reg expr) in that insn.  */
        rtx list = mem_ptr->stores;

        for ( ; list != NULL_RTX; list = XEXP (list, 1))

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]