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]

Re: *_BY_PIECES_P flaw

From: David Edelsohn <dje at watson dot ibm dot com>
To: James E Wilson <wilson at specifixinc dot com>
Cc: Jakub Jelinek <jakub at redhat dot com>, gcc-patches at gcc dot gnu dot org
Date: Wed, 11 Aug 2004 14:34:25 -0400
Subject: Re: *_BY_PIECES_P flaw
References: <20040805134718.GV8296@devserv.devel.redhat.com> <4113F008.2000107@specifixinc.com> <200408090156.i791u0D28492@makai.watson.ibm.com> <1092174155.6313.25.camel@aretha.corp.specifixinc.com>

>>>>> James E Wilson writes:

Jim> If align == GET_MODE_ALIGNMENT (mode), then you run through the loop and
Jim> hit the MAX which will assign align the same value it already has.   We
Jim> should avoid that somehow.

Jim> You are still using MOVE_MAX here.  I think this should be
Jim> MOVE_MAX_PIECES or STORE_MAX_PIECES depending on the context.  Jakub's
Jim> patch has already removed the other use of MOVE_MAX, so we should get
Jim> rid of this one too and be consistent.

Jim> This needs a ChangeLog entry and it needs to be sent to gcc-patches.

	Appended is a revised patch.  I changed the comparison for the
largest alignment to the alignment for the mode corresponding to
MOVE_MAX_PIECES (or STORE_MAX_PIECES).  That is the alignment that will be
compared later when the algorithm tries to perform the largest move.

	Directly testing if align == GET_MODE_ALIGN (mode) seems like the
most straight-forward way to address your other concern to avoid a loop
computing the original value.  Did you have a specific solution in mind?
Did you envision a specific case for that exact value, like the following?

	if (align > GET_MODE_ALIGNMENT (tmode))
	  align = GET_MODE_ALIGNMENT (tmode);
	else if (align == GET_MODE_ALIGNMENT)
	  ;
	else
	  ...

Thanks, David


	* expr.c (move_by_pieces): Set alignment for move to minimum of
	MOVE_MAX_PIECES mode alignment and the largest non-slow mode
	alignment, but not less than the original alignment.
	(move_by_pieces_ninsns): Same.
	(can_store_by_pieces): Similar for store with STORE_MAX_PIECES.
	(store_by_pieces_1): Same.

Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.701
diff -c -p -r1.701 expr.c
*** expr.c	10 Aug 2004 13:28:11 -0000	1.701
--- expr.c	11 Aug 2004 17:48:40 -0000
*************** move_by_pieces (rtx to, rtx from, unsign
*** 925,933 ****
  	data.to_addr = copy_addr_to_reg (to_addr);
      }
  
!   if (! SLOW_UNALIGNED_ACCESS (word_mode, align)
!       || align > MOVE_MAX * BITS_PER_UNIT || align >= BIGGEST_ALIGNMENT)
!     align = MOVE_MAX * BITS_PER_UNIT;
  
    /* First move what we can in the largest integer mode, then go to
       successively smaller modes.  */
--- 925,946 ----
  	data.to_addr = copy_addr_to_reg (to_addr);
      }
  
!   tmode = mode_for_size (MOVE_MAX_PIECES * BITS_PER_UNIT, MODE_INT, 1);
!   if (align >= GET_MODE_ALIGNMENT (tmode))
!     align = GET_MODE_ALIGNMENT (tmode);
!   else
!     {
!       enum machine_mode xmode;
! 
!       for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT), xmode = tmode;
! 	   tmode != VOIDmode;
! 	   xmode = tmode, tmode = GET_MODE_WIDER_MODE (tmode))
! 	if (GET_MODE_SIZE (tmode) > MOVE_MAX_PIECES
! 	    || SLOW_UNALIGNED_ACCESS (tmode, align))
! 	  break;
! 
!       align = MAX (align, GET_MODE_ALIGNMENT (xmode));
!     }
  
    /* First move what we can in the largest integer mode, then go to
       successively smaller modes.  */
*************** move_by_pieces_ninsns (unsigned HOST_WID
*** 992,1005 ****
  		       unsigned int max_size)
  {
    unsigned HOST_WIDE_INT n_insns = 0;
  
!   if (! SLOW_UNALIGNED_ACCESS (word_mode, align)
!       || align > MOVE_MAX * BITS_PER_UNIT || align >= BIGGEST_ALIGNMENT)
!     align = MOVE_MAX * BITS_PER_UNIT;
  
    while (max_size > 1)
      {
!       enum machine_mode mode = VOIDmode, tmode;
        enum insn_code icode;
  
        for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT);
--- 1005,1032 ----
  		       unsigned int max_size)
  {
    unsigned HOST_WIDE_INT n_insns = 0;
+   enum machine_mode tmode;
  
!   tmode = mode_for_size (MOVE_MAX_PIECES * BITS_PER_UNIT, MODE_INT, 1);
!   if (align >= GET_MODE_ALIGNMENT (tmode))
!     align = GET_MODE_ALIGNMENT (tmode);
!   else
!     {
!       enum machine_mode tmode, xmode;
! 
!       for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT), xmode = tmode;
! 	   tmode != VOIDmode;
! 	   xmode = tmode, tmode = GET_MODE_WIDER_MODE (tmode))
! 	if (GET_MODE_SIZE (tmode) > MOVE_MAX_PIECES
! 	    || SLOW_UNALIGNED_ACCESS (tmode, align))
! 	  break;
! 
!       align = MAX (align, GET_MODE_ALIGNMENT (xmode));
!     }
  
    while (max_size > 1)
      {
!       enum machine_mode mode = VOIDmode;
        enum insn_code icode;
  
        for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT);
*************** can_store_by_pieces (unsigned HOST_WIDE_
*** 1999,2007 ****
    if (! STORE_BY_PIECES_P (len, align))
      return 0;
  
!   if (! SLOW_UNALIGNED_ACCESS (word_mode, align)
!       || align > MOVE_MAX * BITS_PER_UNIT || align >= BIGGEST_ALIGNMENT)
!     align = MOVE_MAX * BITS_PER_UNIT;
  
    /* We would first store what we can in the largest integer mode, then go to
       successively smaller modes.  */
--- 2026,2047 ----
    if (! STORE_BY_PIECES_P (len, align))
      return 0;
  
!   tmode = mode_for_size (STORE_MAX_PIECES * BITS_PER_UNIT, MODE_INT, 1);
!   if (align >= GET_MODE_ALIGNMENT (tmode))
!     align = GET_MODE_ALIGNMENT (tmode);
!   else
!     {
!       enum machine_mode xmode;
! 
!       for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT), xmode = tmode;
! 	   tmode != VOIDmode;
! 	   xmode = tmode, tmode = GET_MODE_WIDER_MODE (tmode))
! 	if (GET_MODE_SIZE (tmode) > STORE_MAX_PIECES
! 	    || SLOW_UNALIGNED_ACCESS (tmode, align))
! 	  break;
! 
!       align = MAX (align, GET_MODE_ALIGNMENT (xmode));
x!     }
  
    /* We would first store what we can in the largest integer mode, then go to
       successively smaller modes.  */
*************** store_by_pieces_1 (struct store_by_piece
*** 2201,2209 ****
  	data->to_addr = copy_addr_to_reg (to_addr);
      }
  
!   if (! SLOW_UNALIGNED_ACCESS (word_mode, align)
!       || align > MOVE_MAX * BITS_PER_UNIT || align >= BIGGEST_ALIGNMENT)
!     align = MOVE_MAX * BITS_PER_UNIT;
  
    /* First store what we can in the largest integer mode, then go to
       successively smaller modes.  */
--- 2241,2262 ----
  	data->to_addr = copy_addr_to_reg (to_addr);
      }
  
!   tmode = mode_for_size (STORE_MAX_PIECES * BITS_PER_UNIT, MODE_INT, 1);
!   if (align >= GET_MODE_ALIGNMENT (tmode))
!     align = GET_MODE_ALIGNMENT (tmode);
!   else
!     {
!       enum machine_mode xmode;
! 
!       for (tmode = GET_CLASS_NARROWEST_MODE (MODE_INT), xmode = tmode;
! 	   tmode != VOIDmode;
! 	   xmode = tmode, tmode = GET_MODE_WIDER_MODE (tmode))
! 	if (GET_MODE_SIZE (tmode) > STORE_MAX_PIECES
! 	    || SLOW_UNALIGNED_ACCESS (tmode, align))
! 	  break;
! 
!       align = MAX (align, GET_MODE_ALIGNMENT (xmode));
!     }
  
    /* First store what we can in the largest integer mode, then go to
       successively smaller modes.  */

Follow-Ups:
- Re: *_BY_PIECES_P flaw
  - From: James E Wilson

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