RE: [PATCH] genemit: Split insn-emit.cc into ten files.

[Tamar Christina <Tamar.Christina@arm.com>]

> -----Original Message-----
> From: Robin Dapp <rdapp.gcc@gmail.com>
> Sent: Friday, October 13, 2023 4:15 PM
> To: gcc-patches <gcc-patches@gcc.gnu.org>
> Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Tamar
> Christina <Tamar.Christina@arm.com>; rjiejie@linux.alibaba.com
> Subject: Re: [PATCH] genemit: Split insn-emit.cc into ten files.
> 
> > Testsuite is unchanged on all but x86 where, strangely, I saw several
> > illegal instructions in the pch tests.  Those were not reproducible in
> > a second manual test suite run.  I'm just running another full
> > bootstrap and testsuite cycle with the latest trunk.
> 
> Follow-up on the pch tests.  The errors are an artifact of my testing.
> I usually build unpatched without bootstrap and patched with it, comparing
> the testsuite results.
> 
> When bootstrapping unpatched, the same errors occur.  When bootstrapping
> with --with-arch=native we essentially use a vpxor for a memset which is an
> illegal instruction on gc188 of the compile farm.  This might be a known bug
> but I haven't found something in bugzilla.
> 
> In total:  Bootstrap and testsuite unchanged on x86, aarch64 and power10.
> 
> Regarding Tamar's comment:
> 
> > I'll leave the review to Richard, but I think you should adopt the
> > same approach taken by the match.pd split, in that you provide the
> > list of files as an argument to the genemit instead of the number of
> > files.  And if no list is provided it outputs to stdout as it does today.
> 
> I think this would involve rewriting the argument handling for all gen* tools (it
> is shared via gensupport).  Currently, I make use of the callback and just add
> two new options which helps limit the number of changes.  Is stdout so
> valuable from a debugging point of view that changing it would be
> prohibitive?  Of course it's a change but I usually grep through insn-emit.cc
> anyways and that would still be possible.

Debugging here refers to debugging the output of gen*, which usually one does
With small examples when modifying the tool itself.

> I think this would involve rewriting the argument handling for all gen* tools (it
> is shared via gensupport).  Currently, I make use of the callback and just add
> two new options which helps limit the number of changes.  

Hmm why? The same callback you use to consume the listed arguments can be used to
consume the list can it not? I may be wrong, but from what I remember the callback
is called when main can't consume an argv value and it's allowed to eat all remaining input?

But I'll leave it up to Richard. Having to read multiple files to check a change to gen*
seems like a usability issue though.

Regards,
Tamar

> 
> Regards
>  Robin