Georg-Johann Lay <a...@gjlay.de> writes:
> [...]
> Am 13.07.24 um 13:44 schrieb Richard Sandiford:
>> Georg-Johann Lay <a...@gjlay.de> writes:
>>> diff --git a/gcc/read-md.h b/gcc/read-md.h
>>> index 9703551a8fd..ae10b651de1 100644
>>> --- a/gcc/read-md.h
>>> +++ b/gcc/read-md.h
>>> @@ -132,6 +132,38 @@ struct overloaded_name {
>>> overloaded_instance **next_instance_ptr;
>>> };
>>>
>>> +/* Structure for each attribute. */
>>> +
>>> +struct attr_value;
>>> +
>>> +class attr_desc
>>> +{
>>> +public:
>>> + char *name; /* Name of attribute. */
>>> + const char *enum_name; /* Enum name for DEFINE_ENUM_NAME. */
>>> + class attr_desc *next; /* Next attribute. */
>>> + struct attr_value *first_value; /* First value of this attribute. */
>>> + struct attr_value *default_val; /* Default value for this attribute. */
>>> + file_location loc; /* Where in the .md files it occurs. */
>>> + unsigned is_numeric : 1; /* Values of this attribute are
>>> numeric. */
>>> + unsigned is_const : 1; /* Attribute value constant for each
>>> run. */
>>> + unsigned is_special : 1; /* Don't call `write_attr_set'. */
>>> +
>>> + // Print the return type for functions like get_attr_<attribute-name>
>>> + // to stream OUTF, followed by SUFFIX which should be white-space(s).
>>> + void fprint_type (FILE *outf, const char *suffix) const
>>> + {
>>> + if (enum_name)
>>> + fprintf (outf, "enum %s", enum_name);
>>> + else if (! is_numeric)
>>> + fprintf (outf, "enum attr_%s", name);
>>> + else
>>> + fprintf (outf, "int");
>>> +
>>> + fprintf (outf, "%s", suffix);
>>
>> It shouldn't be necessary to emit the enum tag these days. If removing
>
> Hi Richard,
>
> I am not familiar with the gensupport policies, which is the reason why
> the feature is just a suggestion / proposal and not a patch.
> IMO patches should not come from someone like me who has no experience
> in that area; better someone more experienced would take it over.
>
>> it causes anything to break, I think we should fix whatever that breaking
>> thing is. Could you try doing that, as a pre-patch? Or I can give it a
>> go, if you'd rather not.
>
> Yes please.
OK, I pushed b19906a029a to remove the enum tags. The type name is
now stored as a const char * in attr_desc::cxx_type.
>> If we do that, then we can just a return a const char * for the type.
>
> Yes, const char* would be easier. I just didn't know how to alloc one,
> and where. A new const char* property in class attr_desc_would solve
> it.
>
>> And then in turn we can pass a const char * to (f)print_c_condition.
>> The MD reader then wouldn't need to know about attributes.
>>
>> Thanks,
>> Richard
>
> When this feature makes it into GCC, then match_test should behave
> similar, I guess? I.e. support function bodies that return bool.
> I just wasn't sure which caller of fprint_c_condition runs with
> match_test resp. symbol_ref from which context (insn attribute or
> predicate, etc).
Yeah, might be useful for match_test too.
> Thanks for looking into this and for considering it as an extension.
>
> The shortcomings like non-support of pathological comments like
> /* } */ is probably not such a big issue. And fixing it would have
> to touch the md scanner / lexer and have side effects I don't know,
> like on build performance and stability of course. That part could
> be fixed when someone actually needs it.
It looks like we don't support \{ and \}, but that's probably an oversight.
Thanks,
Richard
