Re: [PATCH] !fixup "target/avr: Add defintions of AVR core types"

[Aleksandar Markovic]

On Sat, Feb 8, 2020 at 3:06 PM Philippe Mathieu-Daudé <address@hidden> wrote:
>
> Hi Aleksandar,
>
> On 2/8/20 8:10 AM, Aleksandar Markovic wrote:
> > On Friday, February 7, 2020, Philippe Mathieu-Daudé <address@hidden
> > <mailto:address@hidden>> wrote:
> >
> >     These cores have unresolved review comment:
> >     https://www.mail-archive.com/address@hidden/msg674105.html
> >     <https://www.mail-archive.com/address@hidden/msg674105.html>
> >     https://www.mail-archive.com/address@hidden/msg674259.html
> >     <https://www.mail-archive.com/address@hidden/msg674259.html>
> >     and:
> >     https://www.mail-archive.com/address@hidden/msg676046.html
> >     <https://www.mail-archive.com/address@hidden/msg676046.html>
> >
> >     As we don't want a bad start with this architecture, remove them.
> >
> >
> > I agree with underlying motivation of your fixup.
> >
> > Still, the division of AVR cores into avr1, avr2, ... , xmega7 is here
> > to stay. The reason is that because such coding is a part of ELF header,
> > and this means they will stay forever (as they are approved by some kind
> > of ELF comitee, and are meant not to be ever changed in future).
>
> I am not removing anything ELF related. We don't have any machine using
> CPU avrtiny/avr1/avr2/avr25 so AFAIK I'm simply removing unreviewed dead
> code.
>

The portion of the code that classifies AVR cores is and should remain
as independent as possible of the machine-related code. In other
words, my point is that your fixup  is inadequate. We should not touch
the portion of the code that enumarates AVR core types, except to add
some TODO notes as noted during review. If one wants to remove "dead"
code as defined by you, one should leave just two AVR core types and
also eliminate at least half of "AVRFeatures", which is absurd.

There is much less intrusive solution to this, that will also provide
guarantee that we never emulate the code that we know has some issues,
or support for it is not completed yet.

You should read more carefully previous reviews, and you'll see that
Michael and I agreed that support for "avrtiny" cores will be provided
in some point in future, but this doesn't mean we should delete that
AVR type from the current code.

I hope I am clear enough to you now. :)

Regards,
Aleksandar