This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, RFC]: Next stage1, refactoring: propagating rtx subclasses


Hi!

I'm currently working on the proposed task of replacing rtx objects
(i.e. struct rtx_def) with derived classes. I would like to get some
feedback on this work (it's far from being finished, but basically I
would like to know, whether my modifications are appropriate, e.g. one
may consider that this is "too much" for just refactoring, because
sometimes they involve small modification of semantics).

The attached patch is not well tested, i.e. I bootstrapped and regtested
it only on x86_64, but I'll perform more extensive testing before
submitting the next version.

The key points I would like to ask about:

1. The original task was to replace the rtx type by rtx_insn *, where it
is appropriate. But rtx_insn has several derived classes, such as
rtx_code_label, for example. So I tried to use the most derived type
when possible. Is it OK?

2. Not all of these "type promotions" can be done by just looking at
function callers and callees (and some functions will only be generated
during the build of some rare architecture) and checks already done in
them. In a couple of cases I referred to comments and my general
understanding of code semantics. In one case this actually caused a
regression (in the patch it is fixed, of course), because of somewhat
misleading comment (see "live_label_rtx" function added in patch for
details) The question is - are such changes OK for refactoring (or it
should strictly preserve semantics)?

3. In lra-constraints.c I added a new class rtx_usage_list, which, IMHO,
allows to group the functions which work with usage list in a more
explicit manner and make some conditions more self-explaining. I hope
that Vladimir Makarov (in this case, because it concerns LRA) and other
authors will not object against such "intrusion" into their code (or
would rather tell me what should be fixed in my patch(es), instead of
just refusing to apply it/them). In general, are such changes OK or
should better be avoided?

A couple of questions related to further work:

1. I noticed that emit_insn function, in fact, does two kinds of things:
it can either add it's argument to the chain, or, if the argument is a
pattern, it creates a new instruction based on that pattern. Shouldn't
this logic be separated in the callers?

2. Are there any plans on implementing a better class hierarchy on AST's
("union tree_node" type). I see that C++ FE uses a huge number of macros
(which check TREE_CODE and some boolean flags). Could this be improved
somehow?

-- 
Regards,
    Mikhail Maltsev

Attachment: as_insn.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]