From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18902 invoked by alias); 18 Sep 2002 20:53:26 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 18895 invoked from network); 18 Sep 2002 20:53:25 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 18 Sep 2002 20:53:25 -0000 Received: from ges.redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id CF7013D37; Wed, 18 Sep 2002 16:53:23 -0400 (EDT) Message-ID: <3D88E7C3.6050801@ges.redhat.com> Date: Wed, 18 Sep 2002 13:53:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020824 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Marko Mlinar Cc: gdb-patches@sources.redhat.com Subject: Re: Merging OC gdb with official gdb References: <200209041256.47379.markom@opencores.org> <3D7CC5F0.7040100@ges.redhat.com> <200209161329.37700.markom@opencores.org> <200209171444.53129.markom@opencores.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-09/txt/msg00426.txt.bz2 > Originaly it was written for gdb-5.0, but recently we wanted to join it with > official release, so I coded the macros in gdbarch. Getting there. > gdb/config/tm-or1k.h (or1k architecture macros) > gdb/config/tm-or32.h (or32 implementation target macros) Per, my other e-mail this is a new architecture that doesn't appear to require shared library support and hence shouldn't need the tm.h files. If something needs to be shared between orxxx-tdep.c and other files then it should be declared in orxxx-tdep.h. If you find that you do appear to need to define certain macro's then post a question to check what is going on. Some other notes on or1k-tdep.c: > #include "symcat.h" GDB assumes that an ISO C compiler is being used so "symcat.h" shouldn't be needed. > #define OR1K_IS_GPR(N) ((N) >= 0 && (N) < MAX_GPR_REGS) > #define OR1K_IS_VF(N) ((N) >= MAX_GPR_REGS && (N) < MAX_GPR_REGS + MAX_VF_REGS) Macro's like this can simply be written as functions. GDB's convention turns out to be: static int or1k_gpr_p (int regnum) { return (regnum >= 0 && regnum < MAX_GPR_REGS); } > char *or1k_reg_names[] = { > > /* group 0 - general*/ > "VR", "UPR", "CPUCFGR", "DMMUCFGR", "IMMUCFGR", "DCCFGR", "ICCFGR", "DCFGR", > "PCCFGR", "SPR0_9", "SPR0_10", "SPR0_11", "SPR0_12", "SPR0_13", "SPR0_14", "SPR0_15", > "NPC", "SR", "PPC", "SPR0_19", "SPR0_20", "SPR0_21", "SPR0_22", "SPR0_23", > "SPR0_24", "SPR0_25", "SPR0_26", "SPR0_27", "SPR0_28", "SPR0_29", "SPR0_30", "SPR0_31", > "EPCR0", "EPCR1", "EPCR2", "EPCR3", "EPCR4", "EPCR5", "EPCR6", "EPCR7", > "EPCR8", "EPCR9", "EPCR10", "EPCR11", "EPCR12", "EPCR13", "EPCR14", "EPCR15", > "EEAR0","EEAR1", "EEAR2", "EEAR3", "EEAR4", "EEAR5", "EEAR6", "EEAR7", Please change all the register names to lower case so that they are consistent with all other GDB targets. Should the array be static. > /* Builds and returns register name. */ > > static char tmp_name[16]; > static char * > or1k_spr_register_name (i) > int i; > { GDB assumes ISO C so all K&R functions should be converted to ISO C. This particular function now returns ``const char *''. Can you please check that GDB configured with: --target= --enable-gdb-build-warnings=,-Werror compiles (in particular your file). > int group = i >> SPR_GROUP_SIZE_BITS; > int index = i & (SPR_GROUP_SIZE - 1); > switch (group) > { > /* Names covered in or1k_reg_names. */ > case 0: > > /* Generate upper names. */ > if (index >= SPR_GPR_START) > { > if (index < SPR_VFPR_START) > sprintf (tmp_name, "GPR%i", index - SPR_GPR_START); > else > sprintf (tmp_name, "VFR%i", index - SPR_VFPR_START); > return (char *)&tmp_name; This code makes wrong assumptions about how the function will be used. > static int > do_vf_register (regnum) > int regnum; > { > /* do values for FP (float) regs */ > char *raw_buffer; > > /* doubles extracted from raw hex data */ > double doub, flt; > int inv1, inv3, byte; > > raw_buffer = (char *) alloca (OR1K_VF_REGSIZE); > > /* Get the data in raw format. */ > > if (!frame_register_read (selected_frame, regnum, raw_buffer)) Yes! Many targets incorrectly display the hardware registers instead of the current frame's registers. Suggest passing the relevant frame in as a parameter though. selected_frame will one day go away > error ("can't read register %d (%s)", regnum, REGISTER_NAME (regnum)); > > flt = unpack_double (builtin_type_float, raw_buffer, &inv1); > doub = unpack_double (builtin_type_double, raw_buffer, &inv3); Here use the ieee be/le size specific versions. The code can't rely on float/double being something sensible. > if (inv1) > printf_filtered (" %-5s flt: ", REGISTER_NAME (regnum)); > else > printf_filtered (" %-5s flt:%-17.9g", REGISTER_NAME (regnum), flt); > printf_filtered (inv3 ? " dbl: " : > " dbl: %-24.17g ", doub); > > void > _initialize_or1k_tdep () > { > build_automata (); > register_gdbarch_init (bfd_arch_or32, or1k_gdbarch_init); > > if (TARGET_BYTE_ORDER == BIG_ENDIAN) > tm_print_insn = print_insn_big_or32; > else > tm_print_insn = print_insn_little_or32; > > /* Commands to show and set sprs. */ > add_info ("spr", info_spr_command, "Show information about the spr registers."); This should be ``info or1k spr''. See ppc for an example of how to do this. > add_com ("spr", class_support, spr_command, "Set specified SPR register."); This command shouldn't be needed. set $ = should work. > /* hwatch command. */ > add_com ("hwatch", class_breakpoint, hwatch_command, "Set hardware watch" > "point.\nExample: ($LEA == my_var)&&($LDATA < 50)||($SEA == my_" > "var)&&($SDATA >= 50).\nSee OR1k Architecture document for more" > " info."); I don't think this command is needed. GDB already has hardware watchpoint commands. > /* htrace commands. */ > add_prefix_cmd ("htrace", class_breakpoint, htrace_command, > "Group of commands for handling hardware assisted trace\n\n" > "See OR1k Architecture and gdb for or1k documents for more info.", > &htrace_cmdlist, "htrace ", 0, &cmdlist); > add_cmd ("info", class_breakpoint, htrace_info_command, "Display information about HW trace.", > &htrace_cmdlist); > add_alias_cmd ("i", "info", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("trigger", class_breakpoint, htrace_trigger_command, "Set starting criteria for trace.", > &htrace_cmdlist); > add_alias_cmd ("t", "trigger", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("qualifier", class_breakpoint, htrace_qualifier_command, "Set acquisition qualifier for HW trace.", > &htrace_cmdlist); > add_alias_cmd ("q", "qualifier", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("stop", class_breakpoint, htrace_stop_command, "Set HW trace stopping criteria.", > &htrace_cmdlist); > add_alias_cmd ("s", "stop", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("record", class_breakpoint, htrace_record_command, "Sets data to be recorded when expression occurs.", > &htrace_cmdlist); > add_alias_cmd ("r", "record", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("clear records", class_breakpoint, htrace_clear_records_command, > "Disposes all matchpoints used by records.", &htrace_cmdlist); > add_cmd ("enable", class_breakpoint, htrace_enable_command, "Enables the HW trace.", &htrace_cmdlist); > add_alias_cmd ("e", "enable", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("disable", class_breakpoint, htrace_disable_command, "Disables the HW trace.", &htrace_cmdlist); > add_alias_cmd ("d", "disable", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("rewind", class_breakpoint, htrace_rewind_command, "Clears currently recorded trace data.\n" > "If filename is specified, new trace file is made and any newly collected data\n" > "will be written there.", &htrace_cmdlist); > add_cmd ("print", class_breakpoint, htrace_print_command, > "Prints trace buffer, using current record configuration.\n" > "htrace print [ []]\n" > "htrace print" > , &htrace_cmdlist); > add_alias_cmd ("p", "print", class_breakpoint, 1, &htrace_cmdlist); > add_prefix_cmd ("mode", class_breakpoint, htrace_mode_command, > "Configures the HW trace.\n" > "htrace mode [continuous|suspend]" > , &htrace_mode_cmdlist, "htrace mode ", 0, &htrace_cmdlist); > add_alias_cmd ("m", "mode", class_breakpoint, 1, &htrace_cmdlist); > add_cmd ("continuous", class_breakpoint, htrace_mode_contin_command, > "Set continuous trace mode.\n", &htrace_mode_cmdlist); > add_cmd ("suspend", class_breakpoint, htrace_mode_suspend_command, > "Set suspend trace mode.\n", &htrace_mode_cmdlist); Can I suggest, for the moment, moving this funcitonality out of or1k-tdep.c (to or1k-trace.c?). This is a very significant chunk of work adding many new commands and hence is best separated and considered separatly. > /* Extra functions supported by simulator. */ > add_com ("sim", class_obscure, sim_command, > "Send a extended command to the simulator."); There is already a sim command. See remote-sim.c. Andrew