On Sat, Feb 8, 2020 at 3:06 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > Hi Aleksandar, > > On 2/8/20 8:10 AM, Aleksandar Markovic wrote: > > On Friday, February 7, 2020, Philippe Mathieu-Daudé <phi...@redhat.com > > <mailto:phi...@redhat.com>> wrote: > > > > These cores have unresolved review comment: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg674105.html > > <https://www.mail-archive.com/qemu-devel@nongnu.org/msg674105.html> > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg674259.html > > <https://www.mail-archive.com/qemu-devel@nongnu.org/msg674259.html> > > and: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg676046.html > > <https://www.mail-archive.com/qemu-devel@nongnu.org/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