On Tue, 2013-12-03 at 14:09 +0100, Jan Kratochvil wrote: > The patch seems ready to me, it just waits for the review of: > jankratochvil/ppc64bidir
Yes, the patch seems OK, please just checkin the non-tests parts that don't depend on the ppc64bidir stuff. > On Fri, 29 Nov 2013 11:32:08 +0100, Mark Wielaard wrote: > > Aha, yes of course, "nip" isn't associated with any regno. I think I > > would have used the "nip" name as marker or maybe introduced a > > "fake/special" regno for it. Instead of adding a new pc_register boolean > > field to every register to save a little space, but I guess this works > > too and there aren't so many registers that it really matters. > > It is ugly to add named-exceptions in the code. yeah, I guess if you cannot describe "nip" as part of a Register_Location then in this case special casing on the name of a note item in generic code is ugly. Thanks for adding the various comments. > > OK, but just below this code is another loop over the items/regs which > > contains some FIXMEs that you seem to have fixed now. Can't these loops > > be combined? It currently seems to duplicate the value extraction code > > and seems to depend on the order of notes/items. It would be better if > > it was just one loop that only depended on the regno (and/or pc_register > > flag or special PC name). > > This new Ebl_Core_Item based code is unrelated to the Ebl_Register_Location > code below it. The Ebl_Core_Item code above sets PC register which has no > DWARF register number. The Ebl_Register_Location code below sets LR register > which has either 65 or 108 DWARF register number. > > The Ebl_Register_Location hack there is ugly but it is unrelated to the code > above. Right, by mistake. I was hoping there was a way to make "nip" being part of the Register_Locations. But it isn't. It is described as part of a note item. Which are enumerated separately. Thanks, Mark