[07/25] Lazily and dynamically create i386-linux target descriptions
Commit Message
Instead of using pre-generated target descriptions, this patch
changes GDB to lazily and dynamically create target descriptions
according to the target hardware capability (xcr0 in i386).
This support any combination of target features.
This patch also adds a unit test to make sure dynamically generated
tdesc are identical to these generated from xml files.
gdb:
2017-04-27 Yao Qi <yao.qi@linaro.org>
* i386-linux-tdep.c (i386_linux_read_description): Generate
target description if it doesn't exist.
[GDB_SELF_TEST] (i386_linux_read_description_test): New unit test.
(_initialize_i386_linux_tdep) [GDB_SELF_TEST]: Register unit test.
---
gdb/features/i386/32bit-linux.c | 1 +
gdb/features/i386/32bit-sse.c | 1 +
gdb/i386-linux-tdep.c | 83 ++++++++++++++++++++++++-----------------
gdb/target-descriptions.c | 11 ++++++
4 files changed, 62 insertions(+), 34 deletions(-)
Comments
On 06/12/2017 09:41 AM, Yao Qi wrote:
> Instead of using pre-generated target descriptions, this patch
> changes GDB to lazily and dynamically create target descriptions
> according to the target hardware capability (xcr0 in i386).
> This support any combination of target features.
>
> This patch also adds a unit test to make sure dynamically generated
> tdesc are identical to these generated from xml files.
I don't see the unit test in the patch.
Can you say something about the "regnum = %ld" change? Is it
related to the rest of the patch?
>
> 2017-04-27 Yao Qi <yao.qi@linaro.org>
>
> * i386-linux-tdep.c (i386_linux_read_description): Generate
> target description if it doesn't exist.
> [GDB_SELF_TEST] (i386_linux_read_description_test): New unit test.
> (_initialize_i386_linux_tdep) [GDB_SELF_TEST]: Register unit test.
Leading TAB vs spaces.
> +private:
> + /* The register number to use for the next register we see. */
> + int next_regnum = 0;
> };
"m_" prefix.
Thanks,
Pedro Alves
Pedro Alves <palves@redhat.com> writes:
> I don't see the unit test in the patch.
>
It is moved to patch 08/25.
> Can you say something about the "regnum = %ld" change? Is it
> related to the rest of the patch?
>
Without this change, the lazily generated tdesc doesn't equal to the
original pre-generated tdesc if the register has regnum attribute, like
<reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>
most of the reg doesn't have "regnum" attribute so the number is
allocated sequentially. If the reg has "regnum", it explicitly set the
register number. I'll rewrite the commit log this way,
Instead of using pre-generated target descriptions, this patch
changes GDB to lazily and dynamically create target descriptions
according to the target hardware capability (xcr0 in i386).
This support any combination of target features.
Some reg in XML target description has attribute "regnum" which set the
register number explicitly rather than using the sequentially allocated
number,
<reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>
In order to handle this, this patch also changes the visitor to print
the code to set regnum when reg has attribute "regnum".
2017-06-20 Yao Qi <yao.qi@linaro.org>
* i386-linux-tdep.c: Don't include features/i386/i386-XXX-linux.c.
include features/i386/32bit-XXX.c instead.
(i386_linux_read_description): Generate
target description if it doesn't exist.
* target-descriptions.c (print_c_tdesc::visit): Print code to
set regnum.
(print_c_tdesc) <m_next_regnum>: New field.
* features/i386/32bit-linux.c: Regenerated.
* features/i386/32bit-sse.c: Regenerated.
>>
>> 2017-04-27 Yao Qi <yao.qi@linaro.org>
>>
>> * i386-linux-tdep.c (i386_linux_read_description): Generate
>> target description if it doesn't exist.
>> [GDB_SELF_TEST] (i386_linux_read_description_test): New unit test.
>> (_initialize_i386_linux_tdep) [GDB_SELF_TEST]: Register unit test.
>
> Leading TAB vs spaces.
>
These two lines should be removed.
>> +private:
>> + /* The register number to use for the next register we see. */
>> + int next_regnum = 0;
>> };
>
> "m_" prefix.
I'll fix it.
On 06/20/2017 03:07 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>> Can you say something about the "regnum = %ld" change? Is it
>> related to the rest of the patch?
>>
...
> Some reg in XML target description has attribute "regnum" which set the
> register number explicitly rather than using the sequentially allocated
> number,
>
> <reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>
>
> In order to handle this, this patch also changes the visitor to print
> the code to set regnum when reg has attribute "regnum".
I see.
void visit (const tdesc_reg *reg) override
{
+ if (reg->target_regnum > next_regnum)
+ {
+ printf_unfiltered (" regnum = %ld;\n", reg->target_regnum);
+ next_regnum = reg->target_regnum;
+ }
I guess "reg->target_regnum < next_regnum" shouldn't ever
happen, right? Should we add an assert?
Thanks,
Pedro Alves
@@ -11,6 +11,7 @@ create_feature_i386_32bit_linux (struct target_desc *result, long regnum)
struct tdesc_feature *feature;
feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
+ regnum = 41;
tdesc_create_reg (feature, "orig_eax", regnum++, 1, NULL, 32, "int");
return regnum;
}
@@ -63,6 +63,7 @@ create_feature_i386_32bit_sse (struct target_desc *result, long regnum)
tdesc_add_flag (type, 12, "PM");
tdesc_add_flag (type, 15, "FZ");
+ regnum = 32;
tdesc_create_reg (feature, "xmm0", regnum++, 1, NULL, 128, "vec128");
tdesc_create_reg (feature, "xmm1", regnum++, 1, NULL, 128, "vec128");
tdesc_create_reg (feature, "xmm2", regnum++, 1, NULL, 128, "vec128");
@@ -45,13 +45,14 @@
#include "record-full.h"
#include "linux-record.h"
-#include "features/i386/i386-linux.c"
-#include "features/i386/i386-mmx-linux.c"
-#include "features/i386/i386-mpx-linux.c"
-#include "features/i386/i386-avx-mpx-linux.c"
-#include "features/i386/i386-avx-linux.c"
-#include "features/i386/i386-avx-avx512-linux.c"
-#include "features/i386/i386-avx-mpx-avx512-pku-linux.c"
+
+#include "features/i386/32bit-core.c"
+#include "features/i386/32bit-sse.c"
+#include "features/i386/32bit-linux.c"
+#include "features/i386/32bit-avx.c"
+#include "features/i386/32bit-mpx.c"
+#include "features/i386/32bit-avx512.c"
+#include "features/i386/32bit-pkeys.c"
/* Return non-zero, when the register is in the corresponding register
group. Put the LINUX_ORIG_EAX register in the system group. */
@@ -681,27 +682,50 @@ i386_linux_core_read_xcr0 (bfd *abfd)
const struct target_desc *
i386_linux_read_description (uint64_t xcr0)
{
- switch ((xcr0 & X86_XSTATE_ALL_MASK))
+ if (xcr0 == 0)
+ return NULL;
+
+ static struct target_desc *i386_linux_tdescs \
+ [2/*X87*/][2/*SSE*/][2/*AVX*/][2/*MPX*/][2/*AVX512*/][2/*PKRU*/] = {};
+ struct target_desc **tdesc;
+
+ tdesc = &i386_linux_tdescs[(xcr0 & X86_XSTATE_X87) ? 1 : 0]
+ [(xcr0 & X86_XSTATE_SSE) ? 1 : 0]
+ [(xcr0 & X86_XSTATE_AVX) ? 1 : 0]
+ [(xcr0 & X86_XSTATE_MPX) ? 1 : 0]
+ [(xcr0 & X86_XSTATE_AVX512) ? 1 : 0]
+ [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0];
+
+ if (*tdesc == NULL)
{
- case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
- return tdesc_i386_avx_mpx_avx512_pku_linux;
- case X86_XSTATE_AVX_AVX512_MASK:
- return tdesc_i386_avx_avx512_linux;
- case X86_XSTATE_MPX_MASK:
- return tdesc_i386_mpx_linux;
- case X86_XSTATE_AVX_MPX_MASK:
- return tdesc_i386_avx_mpx_linux;
- case X86_XSTATE_AVX_MASK:
- return tdesc_i386_avx_linux;
- case X86_XSTATE_SSE_MASK:
- return tdesc_i386_linux;
- case X86_XSTATE_X87_MASK:
- return tdesc_i386_mmx_linux;
- default:
- break;
+ *tdesc = allocate_target_description ();
+ set_tdesc_architecture (*tdesc, bfd_scan_arch ("i386"));
+ set_tdesc_osabi (*tdesc, osabi_from_tdesc_string ("GNU/Linux"));
+
+ long regnum = 0;
+
+ if (xcr0 & X86_XSTATE_X87)
+ regnum = create_feature_i386_32bit_core (*tdesc, regnum);
+
+ if (xcr0 & X86_XSTATE_SSE)
+ regnum = create_feature_i386_32bit_sse (*tdesc, regnum);
+
+ regnum = create_feature_i386_32bit_linux (*tdesc, regnum);
+
+ if (xcr0 & X86_XSTATE_AVX)
+ regnum = create_feature_i386_32bit_avx (*tdesc, regnum);
+
+ if (xcr0 & X86_XSTATE_MPX)
+ regnum = create_feature_i386_32bit_mpx (*tdesc, regnum);
+
+ if (xcr0 & X86_XSTATE_AVX512)
+ regnum = create_feature_i386_32bit_avx512 (*tdesc, regnum);
+
+ if (xcr0 & X86_XSTATE_PKRU)
+ regnum = create_feature_i386_32bit_pkeys (*tdesc, regnum);
}
- return NULL;
+ return *tdesc;
}
/* Get Linux/x86 target description from core dump. */
@@ -1092,13 +1116,4 @@ _initialize_i386_linux_tdep (void)
{
gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_LINUX,
i386_linux_init_abi);
-
- /* Initialize the Linux target description. */
- initialize_tdesc_i386_linux ();
- initialize_tdesc_i386_mmx_linux ();
- initialize_tdesc_i386_avx_linux ();
- initialize_tdesc_i386_mpx_linux ();
- initialize_tdesc_i386_avx_mpx_linux ();
- initialize_tdesc_i386_avx_avx512_linux ();
- initialize_tdesc_i386_avx_mpx_avx512_pku_linux ();
}
@@ -2100,6 +2100,12 @@ public:
void visit (const tdesc_reg *reg) override
{
+ if (reg->target_regnum > next_regnum)
+ {
+ printf_unfiltered (" regnum = %ld;\n", reg->target_regnum);
+ next_regnum = reg->target_regnum;
+ }
+
printf_unfiltered (" tdesc_create_reg (feature, \"%s\", regnum++, %d, ",
reg->name, reg->save_restore);
if (reg->group)
@@ -2107,8 +2113,13 @@ public:
else
printf_unfiltered ("NULL, ");
printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
+
+ next_regnum++;
}
+private:
+ /* The register number to use for the next register we see. */
+ int next_regnum = 0;
};
static void