PATCH: Add regcache_raw_supply_part/regcache_raw_collect_part

PATCH: Add regcache_raw_supply_part/regcache_raw_collect_part

[H.J. Lu]

Hi,

Intel AVX saves a 256bit YMM register in lower 128bit and upper 128bit
separately. This patch adds regcache_raw_supply_part and
regcache_raw_collect_part. They will be used in AVX gdb patches . OK
to install?

Thanks.


H.J.
----
2010-02-02  H.J. Lu  <hongjiu.lu@intel.com>

	* regcache.c (regcache_raw_supply_part): New.
	(regcache_raw_collect_part): Likewise.
	* regcache.h (regcache_raw_supply_part): Likewise.
	(regcache_raw_collect_part): Likewise.

diff --git a/gdb/regcache.c b/gdb/regcache.c
index d6f58fe..f01b090 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -874,6 +874,48 @@ regcache_raw_collect (const struct regcache *regcache, int regnum, void *buf)
   memcpy (buf, regbuf, size);
 }
 
+/* Supply SIZE bytes of register REGNUM, whose contents are stored in
+   BUF, to REGCACHE at OFFSET in byte, relative to register REGNUM,
+   and mark register REGNUM with VALID.  */
+
+void
+regcache_raw_supply_part (struct regcache *regcache, int regnum,
+			  const void *buf, int offset, size_t size,
+			  int valid)
+{
+  char *regbuf;
+
+  gdb_assert (regcache != NULL);
+  gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
+  gdb_assert (!regcache->readonly_p);
+
+  regbuf = ((char *) register_buffer (regcache, regnum)) + offset;
+
+  if (buf)
+    memcpy (regbuf, buf, size);
+  else
+    memset (regbuf, 0, size);
+
+  /* Mark the register.  */
+  regcache->register_valid_p[regnum] = valid;
+}
+
+/* Collect SIZE bytes of register REGNUM from REGCACHE from OFFSET in
+   byte, relative to register REGNUM, and store its contents in BUF.  */
+
+void
+regcache_raw_collect_part (const struct regcache *regcache, int regnum,
+			   void *buf, int offset, size_t size)
+{
+  const char *regbuf;
+
+  gdb_assert (regcache != NULL && buf != NULL);
+  gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
+
+  regbuf = ((char *) register_buffer (regcache, regnum)) + offset;
+  memcpy (buf, regbuf, size);
+}
+
 
 /* Special handling for register PC.  */
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index d870960..e31e1f9 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -114,6 +114,13 @@ extern void regcache_raw_supply (struct regcache *regcache,
 				 int regnum, const void *buf);
 extern void regcache_raw_collect (const struct regcache *regcache,
 				  int regnum, void *buf);
+extern void regcache_raw_supply_part (struct regcache *regcache,
+				      int regnum, const void *buf,
+				      int offset, size_t size,
+				      int valid);
+extern void regcache_raw_collect_part (const struct regcache *regcache,
+				       int regnum, void *buf,
+				       int offset, size_t size);
 
 
 /* The type of a register.  This function is slightly more efficient

Re: PATCH: Add regcache_raw_supply_part/regcache_raw_collect_part

[Mark Kettenis]

> Date: Tue, 2 Feb 2010 19:31:31 -0800
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> Hi,
> 
> Intel AVX saves a 256bit YMM register in lower 128bit and upper 128bit
> separately. This patch adds regcache_raw_supply_part and
> regcache_raw_collect_part. They will be used in AVX gdb patches . OK
> to install?

I think introducing these functions would be a mistake.  If you're not
careful, you'll end up with a register cache where part of some
registers is filled with garbage.

I think it is better for the code that fills the register cache, to
collect the pieces and build the full 256-bit value, and then use
regcache_raw_supply() to fill the register cache.

Re: PATCH: Add regcache_raw_supply_part/regcache_raw_collect_part

[H.J. Lu]

On Wed, Feb 3, 2010 at 1:14 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Tue, 2 Feb 2010 19:31:31 -0800
>> From: "H.J. Lu" <hongjiu.lu@intel.com>
>>
>> Hi,
>>
>> Intel AVX saves a 256bit YMM register in lower 128bit and upper 128bit
>> separately. This patch adds regcache_raw_supply_part and
>> regcache_raw_collect_part. They will be used in AVX gdb patches . OK
>> to install?
>
> I think introducing these functions would be a mistake.  If you're not
> careful, you'll end up with a register cache where part of some
> registers is filled with garbage.

They are used

                if ((clear_bv & bit_XSTATE_SSE))
                  regcache_raw_supply_part (regcache, i, NULL,
                                            0, 16, 0);
                else
                  regcache_raw_supply_part (regcache, i,
                                            XSAVE_SSE_ADDR (tdep, regs, i),
                                            0, 16, 0);
                if ((clear_bv & bit_XSTATE_AVX))
                  regcache_raw_supply_part (regcache, i, NULL,
                                            16, 16, 1);
                else
                  regcache_raw_supply_part (regcache, i,
                                            XSAVE_AVXH_ADDR (tdep, regs, i),
                                            16, 16, 1);

I mark regcache as valid after full register is updated. If we get garbage in
regcache, it is a bug.

> I think it is better for the code that fills the register cache, to
> collect the pieces and build the full 256-bit value, and then use
> regcache_raw_supply() to fill the register cache.
>

I can do that with extra memory copy.


-- 
H.J.

Re: PATCH: Add regcache_raw_supply_part/regcache_raw_collect_part

[Daniel Jacobowitz]

On Wed, Feb 03, 2010 at 10:14:04AM +0100, Mark Kettenis wrote:
> I think introducing these functions would be a mistake.  If you're not
> careful, you'll end up with a register cache where part of some
> registers is filled with garbage.

I agree.

-- 
Daniel Jacobowitz
CodeSourcery