Use predicates for RTL objects
Hi, I have posted a patch series [1] for converting some of the RTL code to use predicate macros, as described in the suggestions for beginner GCC projects [2]. Segher was kind enough to give some comments on the initial posting [3]. The code has been bootstrapped natively on x86_64, and I have built cross-compilers for all targets except tilegx which gave build errors even on trunk. The compiler object files are identical with trunk except for *-checksum.o and string tables in build/gen*.o which change from GET_CODE (..) == .. etc to the predicate macro. Not all possible changes have been made yet, I figured I'd send this out to check first. I am hoping one of the maintainers will be able to take some time to review the patches -- a few are quite large but are mechanical. 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) More complex predicates could be left as macros or additional methods could be defined like rtx_def::is_a_nondebug_insn etc. I think this should mostly be an improvement, although the comparisons around INSN may become slightly more confusing: currently, INSN_P (x) is different from is_a (x), and using something like x->is_a_insn () for the former would probably increase confusion. Thanks. [1] https://gcc.gnu.org/ml/gcc-patches/2019-08/msg00327.html [2] https://gcc.gnu.org/projects/beginner.html [3] https://gcc.gnu.org/ml/gcc-patches/2019-08/msg00171.html
Re: Use predicates for RTL objects
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. In "GET_CODE (x) == PLUS" it is clear that what the resulting machine code does is cheap. With "x->is_a (PLUS)", who knows what is happening below the covers! (And "REG_P" and similar are much shorter code to type). Segher
Re: Use predicates for RTL objects
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. > In "GET_CODE (x) == PLUS" it is clear that what the resulting machine > code does is cheap. With "x->is_a (PLUS)", who knows what is happening > below the covers! We already have, for eg, is_a (x), and there are predicate macros whose implementation is more complex than checking the code field. You basically have to trust that it's sensibly implemented, i.e. that it is as efficiently implemented as it can be. I don't think people writing RTL transformations should be overly worried about what machine code their predicates are generating, especially when they're calling the defined API for it. > > (And "REG_P" and similar are much shorter code to type). > That is true for the ones that exist, but there are lots more that don't and it doesn't really make sense to add individual macros for all of them. > > Segher
Re: Use predicates for RTL objects
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. > > In "GET_CODE (x) == PLUS" it is clear that what the resulting machine > > code does is cheap. With "x->is_a (PLUS)", who knows what is happening > > below the covers! > > We already have, for eg, is_a (x), and there are Whis *is* a class. And not all of us are happy with that, but since we don't often have to see it at all, it's not so bad. Having rtx_insn a separate type from rtx is actually useful, btw. > predicate macros whose implementation is more complex than checking the > code field. You basically have to trust that it's sensibly implemented, > i.e. that it is as efficiently implemented as it can be. That's not my point -- my point was that it is *obvious* the way things are now, which is nice. > I don't think > people writing RTL transformations should be overly worried about what > machine code their predicates are generating, especially when > they're calling the defined API for it. The whole *design* of RTL is based around us caring a whole lot. > > (And "REG_P" and similar are much shorter code to type). > > That is true for the ones that exist, but there are lots more that don't > and it doesn't really make sense to add individual macros for all of > them. Yes. So use GET_CODE for those? REG_P is super frequent, it is really handy to have a macro for it. 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. Segher
Re: Use predicates for RTL objects
On Wed, Aug 07, 2019 at 01:05:51PM -0500, 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 the code is semantically a type identifier. Using the common is_a idiom IMO makes it more readable for that reason, but that is a matter of personal opinion, hence my request for comments. > > > > In "GET_CODE (x) == PLUS" it is clear that what the resulting machine > > > code does is cheap. With "x->is_a (PLUS)", who knows what is happening > > > below the covers! > > ... > > predicate macros whose implementation is more complex than checking the > > code field. You basically have to trust that it's sensibly implemented, > > i.e. that it is as efficiently implemented as it can be. > > That's not my point -- my point was that it is *obvious* the way things > are now, which is nice. My reply is pointing out that it is just as (non-)obvious with or without that inline function, if you want to use any of the helper macros. It wouldn't be a reasonable argument to say that INSN_P (x) is obviously efficient while x->is_a (PLUS) is hiding some potentially nasty things inside. There's around 200 uses of GET_CODE being compared to PLUS in the non-target-specific code, so it's not that rare. It would be nice to replace it with something better, but PLUS_P (x) just reads badly to me. > > > I don't think > > people writing RTL transformations should be overly worried about what > > machine code their predicates are generating, especially when > > they're calling the defined API for it. > > The whole *design* of RTL is based around us caring a whole lot. I'm not saying that we don't care about performance. My point is that if you know that what you're checking is just whether this RTX is a PLUS-coded RTX or not, you should not care exactly which machine instructions that check will generate, because you already know that the test should be trivial, and any sensible implementation will be efficient enough, especially after a compiler is done with it. i.e. being sure of getting efficient machine code is not a reason for avoiding macros/inline functions.
Copyright assignment forms
Hi, I would like to begin contributing to the GCC project, and Segher suggested that I should take care of the copyright assignment forms. I am reaching out to obtain them. I am currently unemployed so there's no issue with an employer. Thanks
