Re: [PATCH, generic] New RTL primitive: `define_subst'

[Kirill Yukhin <kirill dot yukhin at gmail dot com>]

Hi guys,
thanks for review. Comments along with updated patch are below.

>> > +     while ((start = strchr (end, '<')) && (end  = strchr (end, '>')))
> Above too when you mention it.
Fixed.

>> Looking at all this, I'm wondering if we shouldn't split out all of this
>> macro/include processing to a separate pass.  Perform the preprocessing
>> once, early, leaving the processed result in the build directory.  Then
>> run the original/traditional rtl reader on that when running the other
>> rtl manipulation passes.
>>
>> The reason being that it's going to become increasingly hard to figure
>> out if the reason for an error is in the macro processing or in the source
>> md file.  Being able to see the macro expansion is going to be important.
>>
>> Of course, another way to get this macro expansion is to leave the macro
>> processing where it is and have another gen program that merely dumps the
>> processed rtl.  It wouldn't be run normally, but a makefile target used
>> for debugging would be sufficient.
>
> Yeah, one or other way to being able to debug what exactly has been
> performed during the iterator expansion is certainly desirable for the
> future.
We actually have internal machinery for dumping MDs with expanded
iterators and substs, but this looks really kinda hack now.
We're going to submit it as subsequent patch after that one
is checked in (there'll be no change in `define_subst' interface) and some
discussion about necessary infrastructure changes is performed.

>> RM's do you have an opinion here?
>
> I'm not against it going in now, it shouldn't be too risky and will
> allow people to start experimenting with it sooner.
>
> It is not the end of the world if it doesn't get applied now on the other
> side, the gen*.c changes aren't that big and invasive and what would be much
> harder to maintain longer term are the actual *.md file changes, which
> aren't being submitted for 4.8.
The main reason we want it as early as possible is to prepare a solid
and validated by community ground for the further possible extensions
which can be added to the i386.md fairly easy by using this machinery.
Though you are right that it can be introduced at the usage point, we
think that define_subst itself is adding functionality that can be
used by other interested developers (e.g., it is possible to squash
i386.md by few hundred lines using `define_subst'). So early adoption
will be beneficial: the more people will look into it, use it, the
more robust and clean it will become.

Other questions/comments?

Thanks, K

Attachment: define_subst-6.patch
Description: Binary data