Patchwork [06/25] Generate c for feature instead of tdesc

login
register
mail settings
Submitter Yao Qi
Date June 12, 2017, 8:41 a.m.
Message ID <1497256916-4958-7-git-send-email-yao.qi@linaro.org>
Download mbox | patch
Permalink /patch/20929/
State New
Headers show

Comments

Yao Qi - June 12, 2017, 8:41 a.m.
This patch changes Makefile and command "maint print c-files" so
that GDB can print c files for features instead target description.
Previously, we feed GDB a target description xml file, which generate
c files including multiple features.

With this patch, in Makefile, we wrap each feature xml file, and
create a temp target description which include only one feature.
Then, adjust the target description printer for them, and print
a c function for each given feature, so that we can use these
c functions later to create target description in a flexible way.

Before GDB prints c-tdesc, we need to set tdesc.  However, this involves
some validations in each gdbarch on these target descriptions, and this
make troubles to generate c file for each target feature when we feed GDB
a target description only including an optional feature (without some
mandatory feature), GDB just reject it, and don't print the c file.

What we need here is to translate xml file to object target_desc, and
translate the object target_desc to c file.  We don't have to involve
gdbarch validation in this process at all, so I modify command
"maint print c-tdesc" to have an optional argument which is the filename
of xml target description.  If the file name is provided, parse the
xml target description, create target_desc object, and print c file from
it.

gdb:

2017-05-22  Yao Qi  <yao.qi@linaro.org>

	* features/Makefile (FEATURE_XMLFILES): New.
	(FEATURE_CFILES): New.
	New rules.
	* features/i386/32bit-avx.c: Generated.
 	* features/i386/32bit-avx512.c: Generated.
 	* features/i386/32bit-core.c: Generated.
 	* features/i386/32bit-linux.c: Generated.
 	* features/i386/32bit-mpx.c: Generated.
 	* features/i386/32bit-pkeys.c: Generated.
 	* features/i386/32bit-sse.c: Generated.
 	* target-descriptions.c: Include algorithm.
	(tdesc_element_visitor): Add method visit_end.
	(print_c_tdesc): Implement visit_end.
	(print_c_tdesc:: m_filename_after_features): Move it to
	protected.
	(print_c_feature): New class.
	(maint_print_c_tdesc_cmd): Use print_c_feature if XML file
	name starts with "i386/32bit-".

gdb/doc:

2017-06-08  Yao Qi  <yao.qi@linaro.org>

	* gdb.texinfo (Maintenance Commands): Document optional
	argument of "maint print c-tdesc".
---
 gdb/doc/gdb.texinfo              |  10 ++--
 gdb/features/Makefile            |  31 ++++++++++-
 gdb/features/i386/32bit-avx.c    |  23 ++++++++
 gdb/features/i386/32bit-avx512.c |  35 ++++++++++++
 gdb/features/i386/32bit-core.c   |  68 +++++++++++++++++++++++
 gdb/features/i386/32bit-linux.c  |  16 ++++++
 gdb/features/i386/32bit-mpx.c    |  53 ++++++++++++++++++
 gdb/features/i386/32bit-pkeys.c  |  16 ++++++
 gdb/features/i386/32bit-sse.c    |  76 ++++++++++++++++++++++++++
 gdb/target-descriptions.c        | 114 +++++++++++++++++++++++++++++++++++----
 10 files changed, 425 insertions(+), 17 deletions(-)
 create mode 100644 gdb/features/i386/32bit-avx.c
 create mode 100644 gdb/features/i386/32bit-avx512.c
 create mode 100644 gdb/features/i386/32bit-core.c
 create mode 100644 gdb/features/i386/32bit-linux.c
 create mode 100644 gdb/features/i386/32bit-mpx.c
 create mode 100644 gdb/features/i386/32bit-pkeys.c
 create mode 100644 gdb/features/i386/32bit-sse.c
Eli Zaretskii - June 12, 2017, 2:48 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Date: Mon, 12 Jun 2017 09:41:37 +0100
> 
> gdb/doc:
> 
> 2017-06-08  Yao Qi  <yao.qi@linaro.org>
> 
> 	* gdb.texinfo (Maintenance Commands): Document optional
> 	argument of "maint print c-tdesc".

Thanks.

> -@kindex maint print c-tdesc
> +@kindex maint print c-tdesc @r{[}@var{file}@r{]}
>  @item maint print c-tdesc
> -Print the current target description (@pxref{Target Descriptions}) as
> -a C source file.  The created source file can be used in @value{GDBN}
> -when an XML parser is not available to parse the description.
> +Print the target description (@pxref{Target Descriptions}) as
> +a C source file.  The target description is got from the optional
                                            ^^^^^^
"is produced", I guess?  Or maybe I don't understand what you mean by
"is got" here.

> +argument @var{file} or the current target description.

I think this should be described in the reverse order:

  By default, the target description is for the current target, but if
  the optional argument @var{file} is provided, that file is used to
  produce the description.

>                                                        The created
> +source file can be used in @value{GDBN} when an XML parser is not
> +available to parse the description.

I also have some clarification requests (and I do realize that the
original text had the same issues):

 . I think we should say a few words regarding the argument FILE: I
   don't believe _any_ file will do in this role, so we should tell
   what kind of files are expected here.
 . "The created source file can be used in GDB ..." -- used how?  I'd
   prefer that we said a sentence or tow about that as well, or had a
   cross-reference to where that is described.

Also, what about a NEWS entry?
Yao Qi - June 13, 2017, 12:07 p.m.
Eli Zaretskii <eliz@gnu.org> writes:

>>                                                        The created
>> +source file can be used in @value{GDBN} when an XML parser is not
>> +available to parse the description.
>
> I also have some clarification requests (and I do realize that the
> original text had the same issues):
>
>  . I think we should say a few words regarding the argument FILE: I
>    don't believe _any_ file will do in this role, so we should tell
>    what kind of files are expected here.

It is an XML target description.

>  . "The created source file can be used in GDB ..." -- used how?  I'd
>    prefer that we said a sentence or tow about that as well, or had a
>    cross-reference to where that is described.

The created source file is built into GDB.  How is the doc and NEWS
entry below?  Note that I removed the last sentence "... when an XML
parser is not available to parse the description", because these created
c files are builtin target descriptions which are used too when an XML
parser is available.

@kindex maint print c-tdesc @r{[}@var{file}@r{]}
@item maint print c-tdesc
Print the target description (@pxref{Target Descriptions}) as
a C source file.  By default, the target description is for the current
target, but if the optional argument @var{file} is provided, that file
is used to produce the description.  The @var{file} is an XML document,
of the form described in @ref{Target Description Format}.  The created
source file is built in @value{GDBN}.

>
> Also, what about a NEWS entry?

* The "maintenance print c-tdesc" command now takes an optional
  argument which is the file name of XML target description.
Eli Zaretskii - June 13, 2017, 2:49 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 13 Jun 2017 13:07:20 +0100
> 
> >  . "The created source file can be used in GDB ..." -- used how?  I'd
> >    prefer that we said a sentence or tow about that as well, or had a
> >    cross-reference to where that is described.
> 
> The created source file is built into GDB.

Sorry, I don't understand: built into GDB how?  The GDB in which the
user runs this command can then turn around and have the produced file
built into it?

> How is the doc and NEWS entry below?

OK, with two comments, see below.

> @kindex maint print c-tdesc @r{[}@var{file}@r{]}
> @item maint print c-tdesc
> Print the target description (@pxref{Target Descriptions}) as
> a C source file.  By default, the target description is for the current
> target, but if the optional argument @var{file} is provided, that file
> is used to produce the description.  The @var{file} is an XML document,
                                                      ^^
"should be"

> of the form described in @ref{Target Description Format}.  The created
> source file is built in @value{GDBN}.
                       ^^
"into"

> > Also, what about a NEWS entry?
> 
> * The "maintenance print c-tdesc" command now takes an optional
>   argument which is the file name of XML target description.

This is OK.

Thanks.
Yao Qi - June 13, 2017, 3:31 p.m.
Eli Zaretskii <eliz@gnu.org> writes:

> Sorry, I don't understand: built into GDB how?  The GDB in which the
> user runs this command can then turn around and have the produced file
> built into it?

Yes,
We feed GDB the XML target description file, let GDB print the c file,
and then include these generated c files in *-tdep.c.  That is, in
gdb/features/Makefile, we have,

%.c: %.xml
        $(GDB) -nx -q -batch \
          -ex "set tdesc filename $<" -ex 'maint print c-tdesc' > $@.tmp
        sh ../../move-if-change $@.tmp $@

it generates gdb/features/*.c files from the corresponding .xml files.
They are included in gdb source, like, in gdb/i386-linux-tdep.c,

#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"
Eli Zaretskii - June 13, 2017, 3:41 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 13 Jun 2017 16:31:16 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Sorry, I don't understand: built into GDB how?  The GDB in which the
> > user runs this command can then turn around and have the produced file
> > built into it?
> 
> Yes,
> We feed GDB the XML target description file, let GDB print the c file,
> and then include these generated c files in *-tdep.c.

But that means the produced file can only be used by rebuilding GDB,
doesn't it?  If so, the text should make this clear, I think.

(In general, since GDB is a compiled program which cannot easily be
modified without rebuilding it, I wonder how useful this feature will
be.  But I'll yield to your expertise on that.)
Yao Qi - June 14, 2017, 4:21 p.m.
Eli Zaretskii <eliz@gnu.org> writes:

> But that means the produced file can only be used by rebuilding GDB,
> doesn't it?  If so, the text should make this clear, I think.
>

Yes.  The last sentence is about it, "The created source file is built
into @value{GDBN}", but I didn't explicitly say GDB needs rebuild.

> (In general, since GDB is a compiled program which cannot easily be
> modified without rebuilding it, I wonder how useful this feature will
> be.  But I'll yield to your expertise on that.)

This command is only used by developers after they add or modify XML
target descriptions.  I don't think GDB users will use this command.
A question in general, do we really need to document these commands,
which are used for GDB development, in GDB user manual?  These
commands are "maint print c-tdesc", "maint check xml-descriptions"
(added by one patch in this series), and "maint selftest".  Can we
remove them from the user manual?
Eli Zaretskii - June 14, 2017, 4:32 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 14 Jun 2017 17:21:36 +0100
> 
> > But that means the produced file can only be used by rebuilding GDB,
> > doesn't it?  If so, the text should make this clear, I think.
> >
> 
> Yes.  The last sentence is about it, "The created source file is built
> into @value{GDBN}", but I didn't explicitly say GDB needs rebuild.

I think we should say that.

> > (In general, since GDB is a compiled program which cannot easily be
> > modified without rebuilding it, I wonder how useful this feature will
> > be.  But I'll yield to your expertise on that.)
> 
> This command is only used by developers after they add or modify XML
> target descriptions.

Then I think we should say that as well.

> A question in general, do we really need to document these commands,
> which are used for GDB development, in GDB user manual?  These
> commands are "maint print c-tdesc", "maint check xml-descriptions"
> (added by one patch in this series), and "maint selftest".  Can we
> remove them from the user manual?

If we remove them, there will be no place where they are documented,
right?  GDB developers are GDB users as well, e.g. I read the manual
quite a lot.

So I see no reason to remove these commands.  We just need to indicate
when a command makes sense only for GDB developers.

Thanks.
Yao Qi - June 15, 2017, 1:19 p.m.
Eli Zaretskii <eliz@gnu.org> writes:

>> Yes.  The last sentence is about it, "The created source file is built
>> into @value{GDBN}", but I didn't explicitly say GDB needs rebuild.
>
> I think we should say that.
>
>> > (In general, since GDB is a compiled program which cannot easily be
>> > modified without rebuilding it, I wonder how useful this feature will
>> > be.  But I'll yield to your expertise on that.)
>> 
>> This command is only used by developers after they add or modify XML
>> target descriptions.
>
> Then I think we should say that as well.
>

I update the paragraph for this command a little bit,

@kindex maint print c-tdesc @r{[}@var{file}@r{]}
@item maint print c-tdesc
Print the target description (@pxref{Target Descriptions}) as
a C source file.  By default, the target description is for the current
target, but if the optional argument @var{file} is provided, that file
is used to produce the description.  The @var{file} should be an XML
document, of the form described in @ref{Target Description Format}.
The created source file is built into @value{GDBN} when @value{GDBN} is
built again.  This command is used by developers after they add or
modify XML target descriptions.

>> A question in general, do we really need to document these commands,
>> which are used for GDB development, in GDB user manual?  These
>> commands are "maint print c-tdesc", "maint check xml-descriptions"
>> (added by one patch in this series), and "maint selftest".  Can we
>> remove them from the user manual?
>
> If we remove them, there will be no place where they are documented,
> right?  GDB developers are GDB users as well, e.g. I read the manual
> quite a lot.
>
> So I see no reason to remove these commands.  We just need to indicate
> when a command makes sense only for GDB developers.

but GDB users are not GDB developers.  Why does user manual documents
some commands users never use?  IMO, these documents should be
documented in GDB internals.
Eli Zaretskii - June 15, 2017, 2:45 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 15 Jun 2017 14:19:25 +0100
> 
> I update the paragraph for this command a little bit,
> 
> @kindex maint print c-tdesc @r{[}@var{file}@r{]}
> @item maint print c-tdesc
> Print the target description (@pxref{Target Descriptions}) as
> a C source file.  By default, the target description is for the current
> target, but if the optional argument @var{file} is provided, that file
> is used to produce the description.  The @var{file} should be an XML
> document, of the form described in @ref{Target Description Format}.
> The created source file is built into @value{GDBN} when @value{GDBN} is
> built again.  This command is used by developers after they add or
> modify XML target descriptions.

Thanks, this is good.

> > If we remove them, there will be no place where they are documented,
> > right?  GDB developers are GDB users as well, e.g. I read the manual
> > quite a lot.
> >
> > So I see no reason to remove these commands.  We just need to indicate
> > when a command makes sense only for GDB developers.
> 
> but GDB users are not GDB developers.  Why does user manual documents
> some commands users never use?

Because I expect GDB developers to look for commands they need to use
in this manual.

> IMO, these documents should be documented in GDB internals.

AFAIU, GDB internals documents the GDB implementation, not commands
and options that are used interactively within GDB.
Pedro Alves - June 20, 2017, 10:59 a.m.
(this is not a full review)

On 06/12/2017 09:41 AM, Yao Qi wrote:
> +$(FEATURE_CFILES): %.c: %.xml.tmp
> +	$(GDB) -nx -q -batch \
> +	  -ex 'maint print c-tdesc $<' > $@.tmp
> +	sh ../../move-if-change $@.tmp $@
> +	rm $<
> +
> +%.xml.tmp: %.xml
> +	echo "<?xml version=\"1.0\"?>" > $@
> +	echo "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" >> $@
> +	echo "<target>" >> $@
> +	echo "  <architecture>" >> $@
> +	if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi;
> +	echo "  </architecture>" >> $@
> +	echo "  <xi:include href=\"$(notdir $<)\"/>" >> $@
> +	echo "</target>" >> $@
> +

Don't we need move-if-change here?

The findstring bits warrants a comment.

> +/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
> +  Original: 32bit-avx512.xml.tmp */

I don't think we should be pointing "Original" at a 
temporary file that does not exist in the repo?

On 06/12/2017 09:41 AM, Yao Qi wrote:
> -  print_c_tdesc v (filename_after_features);
> +  if (strncmp (filename_after_features.c_str(), "i386/32bit-", 11) == 0)

startswith

But again, this looks like needs at least a comment.  I'd
like to see an expanded rationale for this.  It's not
immediately obvious why/what's this special casing for.

> +    {
> +      print_c_feature v (filename_after_features);
> +
> +      tdesc->accept (v);
> +    }
> +  else
> +    {
> +      print_c_tdesc v (filename_after_features);
>  
> -  tdesc->accept (v);
> +      tdesc->accept (v);
> +    }


Thanks,
Pedro Alves
Yao Qi - June 22, 2017, 2:49 p.m.
Pedro Alves <palves@redhat.com> writes:

> On 06/12/2017 09:41 AM, Yao Qi wrote:
>> +$(FEATURE_CFILES): %.c: %.xml.tmp
>> +	$(GDB) -nx -q -batch \
>> +	  -ex 'maint print c-tdesc $<' > $@.tmp
>> +	sh ../../move-if-change $@.tmp $@
>> +	rm $<
>> +
>> +%.xml.tmp: %.xml
>> +	echo "<?xml version=\"1.0\"?>" > $@
>> +	echo "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" >> $@
>> +	echo "<target>" >> $@
>> +	echo "  <architecture>" >> $@
>> +	if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi;
>> +	echo "  </architecture>" >> $@
>> +	echo "  <xi:include href=\"$(notdir $<)\"/>" >> $@
>> +	echo "</target>" >> $@
>> +
>
> Don't we need move-if-change here?
>

move-if-change from what to what?  *.xml.tmp is removed after *.c is
generated.

> The findstring bits warrants a comment.
>

Yes, comment is needed.

>> +/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
>> +  Original: 32bit-avx512.xml.tmp */
>
> I don't think we should be pointing "Original" at a 
> temporary file that does not exist in the repo?

Fixed locally.

>
> On 06/12/2017 09:41 AM, Yao Qi wrote:
>> -  print_c_tdesc v (filename_after_features);
>> +  if (strncmp (filename_after_features.c_str(), "i386/32bit-", 11) == 0)
>
> startswith
>
> But again, this looks like needs at least a comment.  I'd
> like to see an expanded rationale for this.  It's not
> immediately obvious why/what's this special casing for.

OK.  Note that the special case and the findstring in Makefile are
needed during the target description transition.  Once we move all
targets to the flexible target description, we don't need them.
Pedro Alves - June 22, 2017, 3:36 p.m.
On 06/22/2017 03:49 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> > On 06/12/2017 09:41 AM, Yao Qi wrote:
>>> >> +$(FEATURE_CFILES): %.c: %.xml.tmp
>>> >> +	$(GDB) -nx -q -batch \
>>> >> +	  -ex 'maint print c-tdesc $<' > $@.tmp
>>> >> +	sh ../../move-if-change $@.tmp $@
>>> >> +	rm $<
>>> >> +
>>> >> +%.xml.tmp: %.xml
>>> >> +	echo "<?xml version=\"1.0\"?>" > $@
>>> >> +	echo "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" >> $@
>>> >> +	echo "<target>" >> $@
>>> >> +	echo "  <architecture>" >> $@
>>> >> +	if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi;
>>> >> +	echo "  </architecture>" >> $@
>>> >> +	echo "  <xi:include href=\"$(notdir $<)\"/>" >> $@
>>> >> +	echo "</target>" >> $@
>>> >> +
>> >
>> > Don't we need move-if-change here?
>> >
> move-if-change from what to what?  *.xml.tmp is removed after *.c is
> generated.

"*.xml.tmp.tmp" to "*.xml.tmp".  I.e., from $@.tmp to $@, just
like the rule further above.

I may be missing something, but AFAICS, the "%.xml.tmp: %.xml"
rule is writing to the target directly.  If the build
is interrupted in the middle of that rule, we end up with an
incomplete "%.xml.tmp" file left over in the file system.
Then the next time you invoke make, make will think that
the "%.xml.tmp" file is already up to date and won't rebuild it.
As a result, you'll pass a broken xml.tmp file to
"maint print c-tdesc".

Avoiding these issues is the whole point of move-if-change,
by atomically creating the target file.

Thanks,
Pedro Alves
Yao Qi - June 22, 2017, 3:58 p.m.
Pedro Alves <palves@redhat.com> writes:

> "*.xml.tmp.tmp" to "*.xml.tmp".  I.e., from $@.tmp to $@, just
> like the rule further above.
>

Oh, I know what you mean.

> I may be missing something, but AFAICS, the "%.xml.tmp: %.xml"
> rule is writing to the target directly.  If the build
> is interrupted in the middle of that rule, we end up with an
> incomplete "%.xml.tmp" file left over in the file system.
> Then the next time you invoke make, make will think that
> the "%.xml.tmp" file is already up to date and won't rebuild it.
> As a result, you'll pass a broken xml.tmp file to
> "maint print c-tdesc".
>
> Avoiding these issues is the whole point of move-if-change,
> by atomically creating the target file.

move-if-change is needed here.  Let me add it.
Simon Marchi - June 26, 2017, 9:38 p.m.
On 2017-06-12 10:41, Yao Qi wrote:
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 3bc8b5a..28a7e20 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -252,12 +252,39 @@ $(outdir)/%.dat: %.xml number-regs.xsl
> sort-regs.xsl gdbserver-regs.xsl
>  	  $(XSLTPROC) gdbserver-regs.xsl - >> $(outdir)/$*.tmp
>  	sh ../../move-if-change $(outdir)/$*.tmp $(outdir)/$*.dat
> 
> -cfiles: $(CFILES)
> -%.c: %.xml
> +FEATURE_XMLFILES = i386/32bit-core.xml \
> +	i386/32bit-sse.xml \
> +	i386/32bit-linux.xml \
> +	i386/32bit-avx.xml \
> +	i386/32bit-mpx.xml \
> +	i386/32bit-avx512.xml \
> +	i386/32bit-pkeys.xml
> +
> +FEATURE_CFILES = $(patsubst %.xml,%.c,$(FEATURE_XMLFILES))
> +
> +cfiles: $(CFILES) $(FEATURE_CFILES)

If we are going to have different kinds of CFILES, it would be nice to 
rename CFILES (to TDESC_CFILES I suppose) to clarify the purpose of each 
variable.

> +
> +$(CFILES): %.c: %.xml
>  	$(GDB) -nx -q -batch \
>  	  -ex "set tdesc filename $<" -ex 'maint print c-tdesc' > $@.tmp
>  	sh ../../move-if-change $@.tmp $@
> 
> +$(FEATURE_CFILES): %.c: %.xml.tmp
> +	$(GDB) -nx -q -batch \
> +	  -ex 'maint print c-tdesc $<' > $@.tmp
> +	sh ../../move-if-change $@.tmp $@
> +	rm $<
> +
> +%.xml.tmp: %.xml
> +	echo "<?xml version=\"1.0\"?>" > $@
> +	echo "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" >> $@
> +	echo "<target>" >> $@
> +	echo "  <architecture>" >> $@
> +	if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi;

Should this be

   test -n "$(findstring ...)"

Quotes would be important here, if findstring returns en empty string,

> +	echo "  </architecture>" >> $@
> +	echo "  <xi:include href=\"$(notdir $<)\"/>" >> $@
> +	echo "</target>" >> $@
> +

Instead of creating this temporary file, wouldn't it be simpler to make 
"maint print c-tdesc" (or a new "maint print c-feature") take directly a 
path to a feature XML, and generate the C from that?
> @@ -2033,38 +2040,123 @@ public:
>      printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
>    }
> 
> +protected:
> +  std::string m_filename_after_features;
> +
>  private:
>    char *m_function;
> -  std::string m_filename_after_features;
>    bool m_printed_field_type = false;
>    bool m_printed_type = false;
>  };
> 
> +class print_c_feature : public print_c_tdesc

I am really not convinced that such an inheritance is appropriate here.  
To make print_c_feature inherit from print_c_tdesc, we should be able to 
say that a print_c_feature object _is_ a print_c_tdesc with further 
specialization.  I don't think it's the case here.  If they can share 
some code, I think it would be appropriate for them to have a common 
base class though.

Simon

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9fb70f6..eb35b43 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34676,11 +34676,13 @@  checksum.
 Print the entire architecture configuration.  The optional argument
 @var{file} names the file where the output goes.
 
-@kindex maint print c-tdesc
+@kindex maint print c-tdesc @r{[}@var{file}@r{]}
 @item maint print c-tdesc
-Print the current target description (@pxref{Target Descriptions}) as
-a C source file.  The created source file can be used in @value{GDBN}
-when an XML parser is not available to parse the description.
+Print the target description (@pxref{Target Descriptions}) as
+a C source file.  The target description is got from the optional
+argument @var{file} or the current target description.  The created
+source file can be used in @value{GDBN} when an XML parser is not
+available to parse the description.
 
 @kindex maint print dummy-frames
 @item maint print dummy-frames
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 3bc8b5a..28a7e20 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -252,12 +252,39 @@  $(outdir)/%.dat: %.xml number-regs.xsl sort-regs.xsl gdbserver-regs.xsl
 	  $(XSLTPROC) gdbserver-regs.xsl - >> $(outdir)/$*.tmp
 	sh ../../move-if-change $(outdir)/$*.tmp $(outdir)/$*.dat
 
-cfiles: $(CFILES)
-%.c: %.xml
+FEATURE_XMLFILES = i386/32bit-core.xml \
+	i386/32bit-sse.xml \
+	i386/32bit-linux.xml \
+	i386/32bit-avx.xml \
+	i386/32bit-mpx.xml \
+	i386/32bit-avx512.xml \
+	i386/32bit-pkeys.xml
+
+FEATURE_CFILES = $(patsubst %.xml,%.c,$(FEATURE_XMLFILES))
+
+cfiles: $(CFILES) $(FEATURE_CFILES)
+
+$(CFILES): %.c: %.xml
 	$(GDB) -nx -q -batch \
 	  -ex "set tdesc filename $<" -ex 'maint print c-tdesc' > $@.tmp
 	sh ../../move-if-change $@.tmp $@
 
+$(FEATURE_CFILES): %.c: %.xml.tmp
+	$(GDB) -nx -q -batch \
+	  -ex 'maint print c-tdesc $<' > $@.tmp
+	sh ../../move-if-change $@.tmp $@
+	rm $<
+
+%.xml.tmp: %.xml
+	echo "<?xml version=\"1.0\"?>" > $@
+	echo "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" >> $@
+	echo "<target>" >> $@
+	echo "  <architecture>" >> $@
+	if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi;
+	echo "  </architecture>" >> $@
+	echo "  <xi:include href=\"$(notdir $<)\"/>" >> $@
+	echo "</target>" >> $@
+
 # Other dependencies.
 $(outdir)/arm/arm-with-iwmmxt.dat: arm/arm-core.xml arm/xscale-iwmmxt.xml
 $(outdir)/i386/i386.dat: i386/32bit-core.xml i386/32bit-sse.xml
diff --git a/gdb/features/i386/32bit-avx.c b/gdb/features/i386/32bit-avx.c
new file mode 100644
index 0000000..53a939b
--- /dev/null
+++ b/gdb/features/i386/32bit-avx.c
@@ -0,0 +1,23 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-avx.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+create_feature_i386_32bit_avx (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
+  tdesc_create_reg (feature, "ymm0h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm1h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm2h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm3h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm4h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm5h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm6h", regnum++, 1, NULL, 128, "uint128");
+  tdesc_create_reg (feature, "ymm7h", regnum++, 1, NULL, 128, "uint128");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-avx512.c b/gdb/features/i386/32bit-avx512.c
new file mode 100644
index 0000000..9bbf392
--- /dev/null
+++ b/gdb/features/i386/32bit-avx512.c
@@ -0,0 +1,35 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-avx512.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+create_feature_i386_32bit_avx512 (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512");
+  struct tdesc_type *field_type;
+  field_type = tdesc_named_type (feature, "uint128");
+  tdesc_create_vector (feature, "v2ui128", field_type, 2);
+
+  tdesc_create_reg (feature, "k0", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k1", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k2", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k3", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k4", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k5", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k6", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "k7", regnum++, 1, NULL, 64, "uint64");
+  tdesc_create_reg (feature, "zmm0h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm1h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm2h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm3h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm4h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm5h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm6h", regnum++, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm7h", regnum++, 1, NULL, 256, "v2ui128");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-core.c b/gdb/features/i386/32bit-core.c
new file mode 100644
index 0000000..c5f0fca
--- /dev/null
+++ b/gdb/features/i386/32bit-core.c
@@ -0,0 +1,68 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-core.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+create_feature_i386_32bit_core (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
+  struct tdesc_type *field_type;
+  struct tdesc_type *type;
+  type = tdesc_create_flags (feature, "i386_eflags", 4);
+  tdesc_add_flag (type, 0, "CF");
+  tdesc_add_flag (type, 1, "");
+  tdesc_add_flag (type, 2, "PF");
+  tdesc_add_flag (type, 4, "AF");
+  tdesc_add_flag (type, 6, "ZF");
+  tdesc_add_flag (type, 7, "SF");
+  tdesc_add_flag (type, 8, "TF");
+  tdesc_add_flag (type, 9, "IF");
+  tdesc_add_flag (type, 10, "DF");
+  tdesc_add_flag (type, 11, "OF");
+  tdesc_add_flag (type, 14, "NT");
+  tdesc_add_flag (type, 16, "RF");
+  tdesc_add_flag (type, 17, "VM");
+  tdesc_add_flag (type, 18, "AC");
+  tdesc_add_flag (type, 19, "VIF");
+  tdesc_add_flag (type, 20, "VIP");
+  tdesc_add_flag (type, 21, "ID");
+
+  tdesc_create_reg (feature, "eax", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "ecx", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "edx", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "ebx", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "esp", regnum++, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "ebp", regnum++, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "esi", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "edi", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "eip", regnum++, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "eflags", regnum++, 1, NULL, 32, "i386_eflags");
+  tdesc_create_reg (feature, "cs", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "ss", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "ds", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "es", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "fs", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "gs", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "st0", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st1", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st2", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st3", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st4", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st5", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st6", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "st7", regnum++, 1, NULL, 80, "i387_ext");
+  tdesc_create_reg (feature, "fctrl", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fstat", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "ftag", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fiseg", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fioff", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "foseg", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fooff", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fop", regnum++, 1, "float", 32, "int");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-linux.c b/gdb/features/i386/32bit-linux.c
new file mode 100644
index 0000000..3f7bfe7
--- /dev/null
+++ b/gdb/features/i386/32bit-linux.c
@@ -0,0 +1,16 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-linux.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+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");
+  tdesc_create_reg (feature, "orig_eax", regnum++, 1, NULL, 32, "int");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-mpx.c b/gdb/features/i386/32bit-mpx.c
new file mode 100644
index 0000000..50b627e
--- /dev/null
+++ b/gdb/features/i386/32bit-mpx.c
@@ -0,0 +1,53 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-mpx.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+create_feature_i386_32bit_mpx (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx");
+  struct tdesc_type *field_type;
+  struct tdesc_type *type;
+  type = tdesc_create_struct (feature, "br128");
+  field_type = tdesc_named_type (feature, "uint64");
+  tdesc_add_field (type, "lbound", field_type);
+  field_type = tdesc_named_type (feature, "uint64");
+  tdesc_add_field (type, "ubound_raw", field_type);
+
+  type = tdesc_create_struct (feature, "_bndstatus");
+  tdesc_set_struct_size (type, 8);
+  tdesc_add_bitfield (type, "bde", 2, 31);
+  tdesc_add_bitfield (type, "error", 0, 1);
+
+  type = tdesc_create_union (feature, "status");
+  field_type = tdesc_named_type (feature, "data_ptr");
+  tdesc_add_field (type, "raw", field_type);
+  field_type = tdesc_named_type (feature, "_bndstatus");
+  tdesc_add_field (type, "status", field_type);
+
+  type = tdesc_create_struct (feature, "_bndcfgu");
+  tdesc_set_struct_size (type, 8);
+  tdesc_add_bitfield (type, "base", 12, 31);
+  tdesc_add_bitfield (type, "reserved", 2, 11);
+  tdesc_add_bitfield (type, "preserved", 1, 1);
+  tdesc_add_bitfield (type, "enabled", 0, 0);
+
+  type = tdesc_create_union (feature, "cfgu");
+  field_type = tdesc_named_type (feature, "data_ptr");
+  tdesc_add_field (type, "raw", field_type);
+  field_type = tdesc_named_type (feature, "_bndcfgu");
+  tdesc_add_field (type, "config", field_type);
+
+  tdesc_create_reg (feature, "bnd0raw", regnum++, 1, NULL, 128, "br128");
+  tdesc_create_reg (feature, "bnd1raw", regnum++, 1, NULL, 128, "br128");
+  tdesc_create_reg (feature, "bnd2raw", regnum++, 1, NULL, 128, "br128");
+  tdesc_create_reg (feature, "bnd3raw", regnum++, 1, NULL, 128, "br128");
+  tdesc_create_reg (feature, "bndcfgu", regnum++, 1, NULL, 64, "cfgu");
+  tdesc_create_reg (feature, "bndstatus", regnum++, 1, NULL, 64, "status");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-pkeys.c b/gdb/features/i386/32bit-pkeys.c
new file mode 100644
index 0000000..afb4958
--- /dev/null
+++ b/gdb/features/i386/32bit-pkeys.c
@@ -0,0 +1,16 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-pkeys.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+create_feature_i386_32bit_pkeys (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys");
+  tdesc_create_reg (feature, "pkru", regnum++, 1, NULL, 32, "uint32");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-sse.c b/gdb/features/i386/32bit-sse.c
new file mode 100644
index 0000000..9aa7d3e
--- /dev/null
+++ b/gdb/features/i386/32bit-sse.c
@@ -0,0 +1,76 @@ 
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-sse.xml.tmp */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+static int
+create_feature_i386_32bit_sse (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
+  struct tdesc_type *field_type;
+  field_type = tdesc_named_type (feature, "ieee_single");
+  tdesc_create_vector (feature, "v4f", field_type, 4);
+
+  field_type = tdesc_named_type (feature, "ieee_double");
+  tdesc_create_vector (feature, "v2d", field_type, 2);
+
+  field_type = tdesc_named_type (feature, "int8");
+  tdesc_create_vector (feature, "v16i8", field_type, 16);
+
+  field_type = tdesc_named_type (feature, "int16");
+  tdesc_create_vector (feature, "v8i16", field_type, 8);
+
+  field_type = tdesc_named_type (feature, "int32");
+  tdesc_create_vector (feature, "v4i32", field_type, 4);
+
+  field_type = tdesc_named_type (feature, "int64");
+  tdesc_create_vector (feature, "v2i64", field_type, 2);
+
+  struct tdesc_type *type;
+  type = tdesc_create_union (feature, "vec128");
+  field_type = tdesc_named_type (feature, "v4f");
+  tdesc_add_field (type, "v4_float", field_type);
+  field_type = tdesc_named_type (feature, "v2d");
+  tdesc_add_field (type, "v2_double", field_type);
+  field_type = tdesc_named_type (feature, "v16i8");
+  tdesc_add_field (type, "v16_int8", field_type);
+  field_type = tdesc_named_type (feature, "v8i16");
+  tdesc_add_field (type, "v8_int16", field_type);
+  field_type = tdesc_named_type (feature, "v4i32");
+  tdesc_add_field (type, "v4_int32", field_type);
+  field_type = tdesc_named_type (feature, "v2i64");
+  tdesc_add_field (type, "v2_int64", field_type);
+  field_type = tdesc_named_type (feature, "uint128");
+  tdesc_add_field (type, "uint128", field_type);
+
+  type = tdesc_create_flags (feature, "i386_mxcsr", 4);
+  tdesc_add_flag (type, 0, "IE");
+  tdesc_add_flag (type, 1, "DE");
+  tdesc_add_flag (type, 2, "ZE");
+  tdesc_add_flag (type, 3, "OE");
+  tdesc_add_flag (type, 4, "UE");
+  tdesc_add_flag (type, 5, "PE");
+  tdesc_add_flag (type, 6, "DAZ");
+  tdesc_add_flag (type, 7, "IM");
+  tdesc_add_flag (type, 8, "DM");
+  tdesc_add_flag (type, 9, "ZM");
+  tdesc_add_flag (type, 10, "OM");
+  tdesc_add_flag (type, 11, "UM");
+  tdesc_add_flag (type, 12, "PM");
+  tdesc_add_flag (type, 15, "FZ");
+
+  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");
+  tdesc_create_reg (feature, "xmm3", regnum++, 1, NULL, 128, "vec128");
+  tdesc_create_reg (feature, "xmm4", regnum++, 1, NULL, 128, "vec128");
+  tdesc_create_reg (feature, "xmm5", regnum++, 1, NULL, 128, "vec128");
+  tdesc_create_reg (feature, "xmm6", regnum++, 1, NULL, 128, "vec128");
+  tdesc_create_reg (feature, "xmm7", regnum++, 1, NULL, 128, "vec128");
+  tdesc_create_reg (feature, "mxcsr", regnum++, 1, "vector", 32, "i386_mxcsr");
+  return regnum;
+}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index ac19792..fceab5f 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -34,6 +34,7 @@ 
 #include "gdb_obstack.h"
 #include "hashtab.h"
 #include "inferior.h"
+#include <algorithm>
 
 class tdesc_element_visitor
 {
@@ -42,6 +43,8 @@  public:
   virtual void visit_end (const target_desc *e) = 0;
 
   virtual void visit (const tdesc_feature *e) = 0;
+  virtual void visit_end (const tdesc_feature *e) = 0;
+
   virtual void visit (const tdesc_type *e) = 0;
   virtual void visit (const tdesc_reg *e) = 0;
 };
@@ -309,8 +312,9 @@  public:
       {
 	reg->accept (v);
       }
-  }
 
+    v.visit_end (this);
+  }
 } *tdesc_feature_p;
 DEF_VEC_P(tdesc_feature_p);
 
@@ -1875,6 +1879,9 @@  public:
 		       e->name);
   }
 
+  void visit_end (const tdesc_feature *e) override
+  {}
+
   void visit_end (const target_desc *e) override
   {
     printf_unfiltered ("\n  tdesc_%s = result;\n", m_function);
@@ -2033,38 +2040,123 @@  public:
     printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
   }
 
+protected:
+  std::string m_filename_after_features;
+
 private:
   char *m_function;
-  std::string m_filename_after_features;
   bool m_printed_field_type = false;
   bool m_printed_type = false;
 };
 
+class print_c_feature : public print_c_tdesc
+{
+public:
+  print_c_feature (std::string &file)
+    : print_c_tdesc (file)
+  {
+    /* Trim ".tmp".  */
+    auto const pos = m_filename_after_features.find_last_of ('.');
+
+    m_filename_after_features = m_filename_after_features.substr (0, pos);
+  }
+
+  void visit (const target_desc *e) override
+  {
+    printf_unfiltered ("#include \"defs.h\"\n");
+    printf_unfiltered ("#include \"osabi.h\"\n");
+    printf_unfiltered ("#include \"target-descriptions.h\"\n");
+    printf_unfiltered ("\n");
+  }
+
+  void visit_end (const target_desc *e) override
+  {}
+
+  void visit (const tdesc_feature *e) override
+  {
+    std::string name (m_filename_after_features);
+
+    auto pos = name.find_first_of ('.');
+
+    name = name.substr (0, pos);
+    std::replace (name.begin (), name.end (), '/', '_');
+    std::replace (name.begin (), name.end (), '-', '_');
+
+    printf_unfiltered ("static int\n");
+    printf_unfiltered ("create_feature_%s ", name.c_str ());
+    printf_unfiltered ("(struct target_desc *result, long regnum)\n");
+
+    printf_unfiltered ("{\n");
+    printf_unfiltered ("  struct tdesc_feature *feature;\n");
+    printf_unfiltered ("\n  feature = tdesc_create_feature (result, \"%s\");\n",
+		       e->name);
+  }
+
+  void visit_end (const tdesc_feature *e) override
+  {
+    printf_unfiltered ("  return regnum;\n");
+    printf_unfiltered ("}\n");
+  }
+
+  void visit (const tdesc_reg *reg) override
+  {
+    printf_unfiltered ("  tdesc_create_reg (feature, \"%s\", regnum++, %d, ",
+		       reg->name, reg->save_restore);
+    if (reg->group)
+      printf_unfiltered ("\"%s\", ", reg->group);
+    else
+      printf_unfiltered ("NULL, ");
+    printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
+  }
+
+};
+
 static void
 maint_print_c_tdesc_cmd (char *args, int from_tty)
 {
   const struct target_desc *tdesc;
+  const char *filename;
+
+  if (args == NULL)
+    {
+      /* Use the global target-supplied description, not the current
+	 architecture's.  This lets a GDB for one architecture generate C
+	 for another architecture's description, even though the gdbarch
+	 initialization code will reject the new description.  */
+      tdesc = current_target_desc;
+      filename = target_description_filename;
+    }
+  else
+    {
+      /* Use the target description from the XML file.  */
+      filename = args;
+      tdesc = file_read_description_xml (filename);
+    }
 
-  /* Use the global target-supplied description, not the current
-     architecture's.  This lets a GDB for one architecture generate C
-     for another architecture's description, even though the gdbarch
-     initialization code will reject the new description.  */
-  tdesc = current_target_desc;
   if (tdesc == NULL)
     error (_("There is no target description to print."));
 
-  if (target_description_filename == NULL)
+  if (filename == NULL)
     error (_("The current target description did not come from an XML file."));
 
-  std::string filename_after_features (target_description_filename);
+  std::string filename_after_features (filename);
   auto loc = filename_after_features.rfind ("/features/");
 
   if (loc != std::string::npos)
     filename_after_features = filename_after_features.substr (loc + 10);
 
-  print_c_tdesc v (filename_after_features);
+  if (strncmp (filename_after_features.c_str(), "i386/32bit-", 11) == 0)
+    {
+      print_c_feature v (filename_after_features);
+
+      tdesc->accept (v);
+    }
+  else
+    {
+      print_c_tdesc v (filename_after_features);
 
-  tdesc->accept (v);
+      tdesc->accept (v);
+    }
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */