From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-14878-listarch-gdb-patches=sourceware.cygnus.com@sources.redhat.com>
Received: (qmail 6289 invoked by alias); 27 Apr 2002 01:45:12 -0000
Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm
Precedence: bulk
List-Subscribe: <mailto:gdb-patches-subscribe@sources.redhat.com>
List-Archive: <http://sources.redhat.com/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sources.redhat.com>
List-Help: <mailto:gdb-patches-help@sources.redhat.com>, <http://sources.redhat.com/ml/#faqs>
Sender: gdb-patches-owner@sources.redhat.com
Received: (qmail 6281 invoked from network); 27 Apr 2002 01:45:09 -0000
Received: from unknown (HELO cygnus.com) (205.180.83.203)
  by sources.redhat.com with SMTP; 27 Apr 2002 01:45:09 -0000
Received: from localhost.redhat.com (remus.sfbay.redhat.com [172.16.27.252])
	by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id SAA01538;
	Fri, 26 Apr 2002 18:45:05 -0700 (PDT)
Received: by localhost.redhat.com (Postfix, from userid 469)
	id B44B310A8C; Fri, 26 Apr 2002 21:44:38 -0400 (EDT)
From: Elena Zannoni <ezannoni@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Message-ID: <15562.646.506619.140778@localhost.redhat.com>
Date: Fri, 26 Apr 2002 18:45:00 -0000
To: Kevin Buettner <kevinb@redhat.com>
Cc: Elena Zannoni <ezannoni@redhat.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] Altivec ABI patches
In-Reply-To: <1020426230236.ZM31350@localhost.localdomain>
References: <15561.48188.141526.557982@localhost.redhat.com>
	<1020426230236.ZM31350@localhost.localdomain>
X-SW-Source: 2002-04/txt/msg01099.txt.bz2

Kevin Buettner writes:
 > On Apr 26,  4:44pm, Elena Zannoni wrote:
 > 
 > > As promised, this patch adds the last missing bits of Altivec support.
 > > It handles the new vector types for passing parameters and returning
 > > values.
 > > 
 > > Elena
 > > 
 > > 2002-04-26  Elena Zannoni  <ezannoni@redhat.com>
 > > 
 > > 	* rs6000-tdep.c (rs6000_extract_return_value,
 > > 	rs6000_store_return_value): Handle returning vectors.
 > > 	(rs6000_gdbarch_init): Use
 > > 	ppc_sysv_abi_broken_use_struct_convention for native sysv cases.
 > > 	* ppc-linux-tdep.c (ppc_sysv_abi_broken_use_struct_convention):
 > > 	New function.
 > > 	(ppc_sysv_abi_use_struct_convention): Deal with functions returning
 > > 	vectors.
 > > 	(ppc_sysv_abi_push_arguments): Handle vector parameters.
 > > 	* ppc-tdep.h (ppc_sysv_abi_broken_use_struct_convention): Export.
 > 
 > These changes look reasonable except for the following change to
 > ppc_sysv_abi_push_arguments():
 > 
 > > +	      if (vreg <= 13)
 > > +		{
 > > +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
 > > +						     + vreg)] = 0;
 > > +		  memcpy (&registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
 > > +						    + vreg)],
 > > +			  v_val_buf, 16);
 > > +		  vreg++;
 > > +		}
 > 
 > Specifically, I'm having trouble understanding the point of this
 > assignment:
 > 
 > > +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
 > > +						     + vreg)] = 0;
 > 
 > Note that the subsequent memcpy() ends up overwriting the memory
 > zero'd by the assignment.  If the assignment does serve some useful
 > purpose, I'd prefer to see memset() used instead.
 > 

It does seem odd, I actually just cut and pasted the code from a few
lines above. I thought there was some reason for it, after all.

	    {
	      *(int *) &registers[REGISTER_BYTE (greg)] = 0;
	      memcpy (&registers[REGISTER_BYTE (greg)], val_buf, 4);
	      greg++;
	    }

Should this go as well, then? Unless it always writes 4 bytes, but the
size of the general register can be bigger?


 > Your patch is approved so long as you address this point.
 > 

Ok.


 > Thanks,
 > 
 > Kevin

Elena