[PATCH 06/25] Generate c for feature instead of tdesc
Simon Marchi
simon.marchi@polymtl.ca
Mon Jun 26 21:38:00 GMT 2017
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
More information about the Gdb-patches
mailing list