| Message ID | 20210831111949.1162022-1-yunqiang.su@cipunited.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] md/define_c_enum: support value assignation | expand |
YunQiang Su <yunqiang.su@cipunited.com> writes: > Currently, the enums from define_c_enum and define_enum can only > has values one by one from 0. > > In fact we can support the behaviour just like C, aka like > (define_enum "mips_isa" [(mips1 1) mips2 (mips32 32) mips32r2]), > then we can get > enum mips_isa { > MIPS_ISA_MIPS1 = 1, > MIPS_ISA_MIPS2 = 2, > MIPS_ISA_MIPS32 = 32, > MIPS_ISA_MIPS32R2 = 33 > }; > > gcc/ChangeLog: > * read-md.c (md_reader::handle_enum): support value assignation. > * doc/md.texi: record define_c_enum value assignation support. > --- > gcc/doc/md.texi | 4 ++++ > gcc/read-md.c | 21 +++++++++++++++++---- > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi > index f8047aefc..2b41cb7fb 100644 > --- a/gcc/doc/md.texi > +++ b/gcc/doc/md.texi > @@ -11074,6 +11074,8 @@ The syntax is as follows: > (define_c_enum "@var{name}" [ > @var{value0} > @var{value1} > + (@var{value32} 32) > + @var{value33} > @dots{} > @var{valuen} > ]) > @@ -11086,6 +11088,8 @@ in @file{insn-constants.h}: > enum @var{name} @{ > @var{value0} = 0, > @var{value1} = 1, > + @var{value32} = 32, > + @var{value33} = 33, > @dots{} > @var{valuen} = @var{n} > @}; > diff --git a/gcc/read-md.c b/gcc/read-md.c > index bb419e0f6..0fbe924d1 100644 > --- a/gcc/read-md.c > +++ b/gcc/read-md.c > @@ -902,7 +902,8 @@ void > md_reader::handle_enum (file_location loc, bool md_p) > { > char *enum_name, *value_name; > - struct md_name name; > + unsigned int cur_value; > + struct md_name name, value; > struct enum_type *def; > struct enum_value *ev; > void **slot; > @@ -928,6 +929,7 @@ md_reader::handle_enum (file_location loc, bool md_p) > *slot = def; > } > > + cur_value = def->num_values; > require_char_ws ('['); > > while ((c = read_skip_spaces ()) != ']') > @@ -937,8 +939,18 @@ md_reader::handle_enum (file_location loc, bool md_p) > error_at (loc, "unterminated construct"); > exit (1); > } > - unread_char (c); > - read_name (&name); > + if (c == '(') > + { > + read_name (&name); > + read_name (&value); > + require_char_ws (')'); > + cur_value = atoi(value.string); Minor formatting nit, sorry, but: should be a space after atoi. OK for trunk with that change, thanks. Richard > + } > + else > + { > + unread_char (c); > + read_name (&name); > + } > > ev = XNEW (struct enum_value); > ev->next = 0; > @@ -954,11 +966,12 @@ md_reader::handle_enum (file_location loc, bool md_p) > ev->name = value_name; > } > ev->def = add_constant (get_md_constants (), value_name, > - md_decimal_string (def->num_values), def); > + md_decimal_string (cur_value), def); > > *def->tail_ptr = ev; > def->tail_ptr = &ev->next; > def->num_values++; > + cur_value++; > } > }
On Tue, Aug 31, 2021 at 4:22 AM YunQiang Su <yunqiang.su@cipunited.com> wrote: > > Currently, the enums from define_c_enum and define_enum can only > has values one by one from 0. > > In fact we can support the behaviour just like C, aka like > (define_enum "mips_isa" [(mips1 1) mips2 (mips32 32) mips32r2]), > then we can get > enum mips_isa { > MIPS_ISA_MIPS1 = 1, > MIPS_ISA_MIPS2 = 2, > MIPS_ISA_MIPS32 = 32, > MIPS_ISA_MIPS32R2 = 33 > }; > > gcc/ChangeLog: > * read-md.c (md_reader::handle_enum): support value assignation. > * doc/md.texi: record define_c_enum value assignation support. > --- > gcc/doc/md.texi | 4 ++++ > gcc/read-md.c | 21 +++++++++++++++++---- > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi > index f8047aefc..2b41cb7fb 100644 > --- a/gcc/doc/md.texi > +++ b/gcc/doc/md.texi > @@ -11074,6 +11074,8 @@ The syntax is as follows: > (define_c_enum "@var{name}" [ > @var{value0} > @var{value1} > + (@var{value32} 32) > + @var{value33} > @dots{} > @var{valuen} > ]) > @@ -11086,6 +11088,8 @@ in @file{insn-constants.h}: > enum @var{name} @{ > @var{value0} = 0, > @var{value1} = 1, > + @var{value32} = 32, > + @var{value33} = 33, > @dots{} > @var{valuen} = @var{n} > @}; > diff --git a/gcc/read-md.c b/gcc/read-md.c > index bb419e0f6..0fbe924d1 100644 > --- a/gcc/read-md.c > +++ b/gcc/read-md.c > @@ -902,7 +902,8 @@ void > md_reader::handle_enum (file_location loc, bool md_p) > { > char *enum_name, *value_name; > - struct md_name name; > + unsigned int cur_value; > + struct md_name name, value; > struct enum_type *def; > struct enum_value *ev; > void **slot; > @@ -928,6 +929,7 @@ md_reader::handle_enum (file_location loc, bool md_p) > *slot = def; > } > > + cur_value = def->num_values; > require_char_ws ('['); > > while ((c = read_skip_spaces ()) != ']') > @@ -937,8 +939,18 @@ md_reader::handle_enum (file_location loc, bool md_p) > error_at (loc, "unterminated construct"); > exit (1); > } > - unread_char (c); > - read_name (&name); > + if (c == '(') > + { > + read_name (&name); > + read_name (&value); > + require_char_ws (')'); > + cur_value = atoi(value.string); We really should be avoiding adding atoi. Yes there are uses already in the source but https://gcc.gnu.org/PR44574 exists to track those uses. Thanks, Andrew > + } > + else > + { > + unread_char (c); > + read_name (&name); > + } > > ev = XNEW (struct enum_value); > ev->next = 0; > @@ -954,11 +966,12 @@ md_reader::handle_enum (file_location loc, bool md_p) > ev->name = value_name; > } > ev->def = add_constant (get_md_constants (), value_name, > - md_decimal_string (def->num_values), def); > + md_decimal_string (cur_value), def); > > *def->tail_ptr = ev; > def->tail_ptr = &ev->next; > def->num_values++; > + cur_value++; > } > } > > -- > 2.30.2 >
Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> 于2021年9月2日周四 上午5:28写道： > > On Tue, Aug 31, 2021 at 4:22 AM YunQiang Su <yunqiang.su@cipunited.com> wrote: > > > > Currently, the enums from define_c_enum and define_enum can only > > has values one by one from 0. > > > > In fact we can support the behaviour just like C, aka like > > (define_enum "mips_isa" [(mips1 1) mips2 (mips32 32) mips32r2]), > > then we can get > > enum mips_isa { > > MIPS_ISA_MIPS1 = 1, > > MIPS_ISA_MIPS2 = 2, > > MIPS_ISA_MIPS32 = 32, > > MIPS_ISA_MIPS32R2 = 33 > > }; > > > > gcc/ChangeLog: > > * read-md.c (md_reader::handle_enum): support value assignation. > > * doc/md.texi: record define_c_enum value assignation support. > > --- > > gcc/doc/md.texi | 4 ++++ > > gcc/read-md.c | 21 +++++++++++++++++---- > > 2 files changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi > > index f8047aefc..2b41cb7fb 100644 > > --- a/gcc/doc/md.texi > > +++ b/gcc/doc/md.texi > > @@ -11074,6 +11074,8 @@ The syntax is as follows: > > (define_c_enum "@var{name}" [ > > @var{value0} > > @var{value1} > > + (@var{value32} 32) > > + @var{value33} > > @dots{} > > @var{valuen} > > ]) > > @@ -11086,6 +11088,8 @@ in @file{insn-constants.h}: > > enum @var{name} @{ > > @var{value0} = 0, > > @var{value1} = 1, > > + @var{value32} = 32, > > + @var{value33} = 33, > > @dots{} > > @var{valuen} = @var{n} > > @}; > > diff --git a/gcc/read-md.c b/gcc/read-md.c > > index bb419e0f6..0fbe924d1 100644 > > --- a/gcc/read-md.c > > +++ b/gcc/read-md.c > > @@ -902,7 +902,8 @@ void > > md_reader::handle_enum (file_location loc, bool md_p) > > { > > char *enum_name, *value_name; > > - struct md_name name; > > + unsigned int cur_value; > > + struct md_name name, value; > > struct enum_type *def; > > struct enum_value *ev; > > void **slot; > > @@ -928,6 +929,7 @@ md_reader::handle_enum (file_location loc, bool md_p) > > *slot = def; > > } > > > > + cur_value = def->num_values; > > require_char_ws ('['); > > > > while ((c = read_skip_spaces ()) != ']') > > @@ -937,8 +939,18 @@ md_reader::handle_enum (file_location loc, bool md_p) > > error_at (loc, "unterminated construct"); > > exit (1); > > } > > - unread_char (c); > > - read_name (&name); > > + if (c == '(') > > + { > > + read_name (&name); > > + read_name (&value); > > + require_char_ws (')'); > > + cur_value = atoi(value.string); > > We really should be avoiding adding atoi. Yes there are uses already It is not user input value, as the value is from our souce code. > in the source but https://gcc.gnu.org/PR44574 exists to track those > uses. > Your problem is still exist: how big range should we support here, for define_enum? > Thanks, > Andrew > > > > + } > > + else > > + { > > + unread_char (c); > > + read_name (&name); > > + } > > > > ev = XNEW (struct enum_value); > > ev->next = 0; > > @@ -954,11 +966,12 @@ md_reader::handle_enum (file_location loc, bool md_p) > > ev->name = value_name; > > } > > ev->def = add_constant (get_md_constants (), value_name, > > - md_decimal_string (def->num_values), def); > > + md_decimal_string (cur_value), def); > > > > *def->tail_ptr = ev; > > def->tail_ptr = &ev->next; > > def->num_values++; > > + cur_value++; > > } > > } > > > > -- > > 2.30.2 > >
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi index f8047aefc..2b41cb7fb 100644 --- a/gcc/doc/md.texi +++ b/gcc/doc/md.texi @@ -11074,6 +11074,8 @@ The syntax is as follows: (define_c_enum "@var{name}" [ @var{value0} @var{value1} + (@var{value32} 32) + @var{value33} @dots{} @var{valuen} ]) @@ -11086,6 +11088,8 @@ in @file{insn-constants.h}: enum @var{name} @{ @var{value0} = 0, @var{value1} = 1, + @var{value32} = 32, + @var{value33} = 33, @dots{} @var{valuen} = @var{n} @}; diff --git a/gcc/read-md.c b/gcc/read-md.c index bb419e0f6..0fbe924d1 100644 --- a/gcc/read-md.c +++ b/gcc/read-md.c @@ -902,7 +902,8 @@ void md_reader::handle_enum (file_location loc, bool md_p) { char *enum_name, *value_name; - struct md_name name; + unsigned int cur_value; + struct md_name name, value; struct enum_type *def; struct enum_value *ev; void **slot; @@ -928,6 +929,7 @@ md_reader::handle_enum (file_location loc, bool md_p) *slot = def; } + cur_value = def->num_values; require_char_ws ('['); while ((c = read_skip_spaces ()) != ']') @@ -937,8 +939,18 @@ md_reader::handle_enum (file_location loc, bool md_p) error_at (loc, "unterminated construct"); exit (1); } - unread_char (c); - read_name (&name); + if (c == '(') + { + read_name (&name); + read_name (&value); + require_char_ws (')'); + cur_value = atoi(value.string); + } + else + { + unread_char (c); + read_name (&name); + } ev = XNEW (struct enum_value); ev->next = 0; @@ -954,11 +966,12 @@ md_reader::handle_enum (file_location loc, bool md_p) ev->name = value_name; } ev->def = add_constant (get_md_constants (), value_name, - md_decimal_string (def->num_values), def); + md_decimal_string (cur_value), def); *def->tail_ptr = ev; def->tail_ptr = &ev->next; def->num_values++; + cur_value++; } }
Currently, the enums from define_c_enum and define_enum can only has values one by one from 0. In fact we can support the behaviour just like C, aka like (define_enum "mips_isa" [(mips1 1) mips2 (mips32 32) mips32r2]), then we can get enum mips_isa { MIPS_ISA_MIPS1 = 1, MIPS_ISA_MIPS2 = 2, MIPS_ISA_MIPS32 = 32, MIPS_ISA_MIPS32R2 = 33 }; gcc/ChangeLog: * read-md.c (md_reader::handle_enum): support value assignation. * doc/md.texi: record define_c_enum value assignation support. --- gcc/doc/md.texi | 4 ++++ gcc/read-md.c | 21 +++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-)