Re: [committed] Re: PATCH: fine-tuning for can_store_by_pieces

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Aug 24, 2007 at 08:09:22PM -0400, Sandra Loosemore wrote:
> Richard Sandiford wrote:
> >Thanks for your patience and all the iterations and testing.
> >This version looks good to me.
> 
> Mark Mitchell wrote:
> >The target-independent changes look fine.
> 
> OK, I've committed the patch.  I found that it collided with this one 
> from Jakub:
> 
> http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01641.html
> 
> but made the obvious correction, and verified that it still builds and 
> that CSiBE results on MIPS do not regress from my previous version. 
> I've attached the final version of the patch.

*************** store_expr (tree exp, rtx target, int ca                                                                         
*** 4507,4513 ****                                                                                                               
                                  str_copy_len, builtin_strncpy_read_str,                                                        
                                  (void *) TREE_STRING_POINTER (exp),                                                            
                                  MEM_ALIGN (target),                                                                            
!                                 exp_len > str_copy_len ? 1 : 0);                                                               
        if (exp_len > str_copy_len)                                                                                              
        clear_storage (dest_mem, GEN_INT (exp_len - str_copy_len),                                                               
                       BLOCK_OP_NORMAL);                                                                                         
--- 4520,4527 ----                                                                                                               
                                  str_copy_len, builtin_strncpy_read_str,                                                        
                                  (void *) TREE_STRING_POINTER (exp),                                                            
                                  MEM_ALIGN (target),                                                                            
!                                 exp_len > str_copy_len ? 1 : 0,                                                                
!                                 false);                                                                                        
        if (exp_len > str_copy_len)                                                                                              
        clear_storage (dest_mem, GEN_INT (exp_len - str_copy_len),                                                               
                       BLOCK_OP_NORMAL);                                                                                         

This is wrong.  You added the memsetp argument to store_by_pieces
as the 6th, before the endp argument, but you are passing
exp_len > str_copy_len ? 1 : 0 as memsetp and false as endp.
This will certainly screw up say
struct A { char a[20]; };
void foo (void)
{
  struct A a = { "abc" };
  bar (&a);
}
because in that case exp_len (20) is bigger than str_copy_len
and so clear_storage is needed for the rest of the array.
If we pass false == 0 as endp, it means the returned endp
will point to the beginning of the array (&a.a[0]), rather
than where store_by_pieces stopped.

	Jakub