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] |
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] |