Re: [PATCH 09/14] Allow nested implications for extensions.

["Christoph Müllner" <christoph.muellner@vrull.eu>]

On Sat, Jul 1, 2023 at 3:33 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 6/30/23 23:22, Christoph Müllner wrote:
> > On Sat, Jul 1, 2023 at 12:24 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
> >>
> >>
> >>
> >> On 6/30/23 15:53, Christoph Müllner wrote:
> >>> On Fri, Jun 30, 2023 at 10:45 PM Jeff Law via Binutils
> >>> <binutils@sourceware.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 6/29/23 11:18, Nathan Huckleberry via Binutils wrote:
> >>>>> Certain extensions require two levels of implications.  For example,
> >>>>> zvkng implies zvkn and zvkn implies zvkned.  Enabling zvkng should also
> >>>>> enable zvkned.
> >>>>>
> >>>>> This patch fixes this behavior.
> >>>>>
> >>>>> Signed-off-by: Nathan Huckleberry <nhuck@google.com>
> >>>>> ---
> >>>>>     bfd/elfxx-riscv.c | 16 ++++++++++++++--
> >>>>>     1 file changed, 14 insertions(+), 2 deletions(-)
> >>>> No problem with the actual code.  I would suggest a comment explicitly
> >>>> stating this only allows two levels rather than arbitrary levels of nesting.
> >>>
> >>> If I read the resulting code correctly, then arbitrary nesting should
> >>> be possible.
> >>> Whenever an implicit subset is found and added, `finished` will be set to false,
> >>> `t` will be reset to the initial value and the inner loop will terminate.
> >>> The outer loop will start over because `finished` is false.
> >>> Do I miss something?
> >> This is making my brain hurt, too late on a Friday afternoon to think.
> >> I should just put it under the debugger (the downside of trying to
> >> review code for a hands-on learner), but I don't have the time right
> >> now.  ISTM there needs to be a worklist of things we've found that we
> >> then need to scan.
> >>
> >> But I'll trust you on this, you're a lot more familiar with this code in
> >> general than I am.
> >>
> >> I wouldn't lose any sleep if you fixed formatting in this code.  We're
> >> supposed to be using GNU style.  So the open curley on the IF statement
> >> should be on its own line, indented twice (which implies the code in the
> >> TRUE arm ought to be reindented) and the close curley should line up
> >> with the open curley.
> >
> > I tested all patches with check_GNU_style.sh and it only found
> > what you have commented here.
> >
> > Since nobody has pushed this series so far, I have updated the code
> > and sent out a v6
> > which can also be found here:
> >    https://github.com/cmuellner/binutils-gdb/tree/riscv-zvk-v6
> I had to fix Nathan's email address in various commits to satisfy the
> commit hooks.  So while the git hashes have changed, the actual source
> contents pushed to the trunk are the same.

Sorry for missing that!
I just downloaded the emails and applied them.
I did go through all commit messages, added the ChangeLog and cleaned
them up, but I did not look at the author line.
Thanks for fixing and pushing!