On 8/7/19 12:05 PM, Segher Boessenkool wrote:
> On Wed, Aug 07, 2019 at 01:39:53PM -0400, Arvind Sankar wrote:
>> On Wed, Aug 07, 2019 at 12:33:53PM -0500, Segher Boessenkool wrote:
>>> On Wed, Aug 07, 2019 at 12:15:29PM -0400, Arvind Sankar wrote:
>>>> I would also like to get some comments on the following idea to make the
>>>> code checks more readable: I am thinking of adding
>>>>    bool rtx_def::is_a (enum rtx_code) const
>>>> This would allow us to make all the rtx_code comparisons more readable
>>>> without having to define individual macros for each.
>>>> i.e.,
>>>>    REG_P (x)                          => x->is_a (REG)
>>>>    GET_CODE (x) == PLUS               => x->is_a (PLUS)
>>>>    GET_CODE (PATTERN (x)) == SEQUENCE => PATTERN (x)->is_a (SEQUENCE)
>>>
>>> That makes things much worse.  Not only is it less readable (IMO), but
>>> the "is_a" idiom is used to check if something is of a certain class,
>>> which is not the case here.
>>
>> Well, the rtx_code *is* kind of a class. It determines what fields of
>> the rtx are valid and what they contain etc.
> 
> It is not a class in the C++ sense.  Confusing this is not useful for
> anyone.
True, but they could be.  When David was working in this space a few
years ago I concluded that the main value in sub-classing the various
RTL operators just wansn't worth the effort.  Instead we focused on
starting to tear apart things like the toplevel objects into rtx_insn
and the like.  THere's little value in treating those as simple RTXs.
INSN_LIST and the like were also ripe for this treatment.

The biggest value in making a real class for the operators things would
be to move the runtime RTL checking into a compile-time check.  But I
couldn't really green light something like that without first completing
the rtx_insn changes.

> 
> 
> If you really want to convert RTL to C++, you should start with getting
> rid of rtx_format and rtx_class, and make REG_P etc. work just as they
> have always done.
Yup.  And continue pushing the rtx_insn bits deeper, tackling INSN_LIST,
etc.

jeff

Reply via email to