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

[Jeff Law <jeffreyalaw@gmail.com>]

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.

Thanks!

jeff