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

Christoph Müllner christoph.muellner@vrull.eu
Sat Jul 1 05:22:24 GMT 2023


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

Thanks,
Christoph

>
>
>
>


More information about the Binutils mailing list