From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26258 invoked by alias); 29 Jan 2012 09:48:47 -0000 Received: (qmail 26248 invoked by uid 22791); 29 Jan 2012 09:48:45 -0000 X-SWARE-Spam-Status: No, hits=-1.2 required=5.0 tests=AWL,BAYES_00,TW_EG,TW_PV,TW_XF X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 29 Jan 2012 09:48:31 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1RrRN8-00070W-JF from Yao_Qi@mentor.com ; Sun, 29 Jan 2012 01:48:30 -0800 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Sun, 29 Jan 2012 01:48:19 -0800 Received: from [127.0.0.1] (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Sun, 29 Jan 2012 01:48:28 -0800 Message-ID: <4F2515EA.9060505@codesourcery.com> Date: Mon, 30 Jan 2012 03:11:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:9.0) Gecko/20111220 Thunderbird/9.0 MIME-Version: 1.0 To: Kevin Buettner CC: Subject: Re: [RFC] Add support for the Renesas rl78 architecture References: <20120125165800.5351c291@mesquite.lan> In-Reply-To: <20120125165800.5351c291@mesquite.lan> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-01/txt/msg00975.txt.bz2 On 01/26/2012 07:58 AM, Kevin Buettner wrote: Kevin, I am not familiar with this arch, so some minor comments on code style only. > +/* Strip bits to form an instruction address. (When fetching a > + 32-bit address from the stack, the high eight bits are garbage. > + This function strips off those unused bits.) */ > +static CORE_ADDR > +rl78_make_instruction_address (CORE_ADDR addr) > +{ > + return addr & 0xffffff; > +} > + > +/* Set / clear bits necessary to make a data address. */ > +static CORE_ADDR > +rl78_make_data_address (CORE_ADDR addr) > +{ > + return (addr & 0xffff) | 0xf0000; > +} > + Why can't we use macro instead of function here? > + > +/* Implement the "pseudo_register_write" gdbarch method. */ > +static void > +rl78_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, > + int reg, const gdb_byte *buffer) > +{ > + if (RL78_BANK0_RP0_REGNUM <= reg && reg <= RL78_BANK3_RP3_REGNUM) > + { > + int raw_regnum = 2 * (reg - RL78_BANK0_RP0_REGNUM) + RL78_BANK0_R0_REGNUM; > + regcache_raw_write (regcache, raw_regnum, buffer); > + regcache_raw_write (regcache, raw_regnum + 1, buffer + 1); > + } > + else if (reg == RL78_SP_REGNUM) > + { > + regcache_raw_write (regcache, RL78_SPL_REGNUM, buffer); > + regcache_raw_write (regcache, RL78_SPH_REGNUM, buffer + 1); > + } > + else if (RL78_X_REGNUM <= reg && reg <= RL78_H_REGNUM) > + { > + ULONGEST psw; > + int bank; > + int raw_regnum; Any empty line is needed here. > + regcache_raw_read_unsigned (regcache, RL78_PSW_REGNUM, &psw); > + bank = ((psw >> 3) & 1) | ((psw >> 4) & 1); > + /* RSB0 is at bit 3; RSBS1 is at bit 5. */ > + raw_regnum = RL78_BANK0_R0_REGNUM + bank * RL78_REGS_PER_BANK > + + (reg - RL78_X_REGNUM); > + regcache_raw_write (regcache, raw_regnum, buffer); > + } > + else if (RL78_AX_REGNUM <= reg && reg <= RL78_HL_REGNUM) > + { > + ULONGEST psw; > + int bank, raw_regnum; Same here. > + regcache_raw_read_unsigned (regcache, RL78_PSW_REGNUM, &psw); > + bank = ((psw >> 3) & 1) | ((psw >> 4) & 1); > + /* RSB0 is at bit 3; RSBS1 is at bit 5. */ > + raw_regnum = RL78_BANK0_R0_REGNUM + bank * RL78_REGS_PER_BANK > + + 2 * (reg - RL78_AX_REGNUM); > + regcache_raw_write (regcache, raw_regnum, buffer); > + regcache_raw_write (regcache, raw_regnum + 1, buffer + 1); > + } > + else > + gdb_assert_not_reached ("invalid pseudo register number"); > +} > + > +/* Implement the "breakpoint_from_pc" gdbarch method. */ > +const gdb_byte * > +rl78_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr) > +{ > + static gdb_byte breakpoint[] = { 0xff }; > + > + /* The above are the bytes required for the BRK instruction. However, > + instructions can be as short as one byte. The simulator looks for > + memory writes to program areas using this pattern, however, and > + implements breakpoints via a different mechanism. */ The code is clear to me, but the comment here is confusing. > + *lenptr = sizeof breakpoint; > + return breakpoint; > +} > + > +/* Function for finding saved registers in a 'struct pv_area'; this > + function is passed to pv_area_scan. > + > + If VALUE is a saved register, ADDR says it was saved at a constant > + offset from the frame base, and SIZE indicates that the whole > + register was saved, record its offset. */ > +static void > +check_for_saved (void *result_untyped, pv_t addr, CORE_ADDR size, pv_t value) > +{ > + struct rl78_prologue *result = (struct rl78_prologue *) result_untyped; > + > + if (value.kind == pvk_register > + && value.k == 0 > + && pv_is_register (addr, RL78_SP_REGNUM) > + && size == register_size (target_gdbarch, value.reg)) > + result->reg_offset[value.reg] = addr.k; > +} > + > +/* Analyze a prologue starting at START_PC, going no further than > + LIMIT_PC. Fill in RESULT as appropriate. */ > +static void > +rl78_analyze_prologue (CORE_ADDR start_pc, > + CORE_ADDR limit_pc, struct rl78_prologue *result) > +{ > + CORE_ADDR pc, next_pc; > + int rn; > + pv_t reg[RL78_NUM_TOTAL_REGS]; > + struct pv_area *stack; > + struct cleanup *back_to; > + CORE_ADDR after_last_frame_setup_insn = start_pc; > + int bank = 0; > + > + memset (result, 0, sizeof (*result)); > + > + for (rn = 0; rn < RL78_NUM_TOTAL_REGS; rn++) > + { > + reg[rn] = pv_register (rn, 0); > + result->reg_offset[rn] = 1; > + } > + > + stack = make_pv_area (RL78_SP_REGNUM, gdbarch_addr_bit (target_gdbarch)); > + back_to = make_cleanup_free_pv_area (stack); > + > + /* The call instruction has saved the return address on the stack. */ > + reg[RL78_SP_REGNUM] = pv_add_constant (reg[RL78_SP_REGNUM], -4); > + pv_area_store (stack, reg[RL78_SP_REGNUM], 4, reg[RL78_PC_REGNUM]); > + > + pc = start_pc; > + while (pc < limit_pc) > + { > + int bytes_read; > + struct rl78_get_opcode_byte_handle opcode_handle; > + RL78_Opcode_Decoded opc; > + > + opcode_handle.pc = pc; > + bytes_read = rl78_decode_opcode (pc, &opc, rl78_get_opcode_byte, > + &opcode_handle); > + next_pc = pc + bytes_read; > + > + if (opc.id == RLO_sel) > + { > + bank = opc.op[1].addend; > + } > + else if (opc.id == RLO_mov > + && opc.op[0].type == RL78_Operand_PreDec > + && opc.op[0].reg == RL78_Reg_SP > + && opc.op[1].type == RL78_Operand_Register) > + { > + int rsrc = (bank * RL78_REGS_PER_BANK) > + + 2 * (opc.op[1].reg - RL78_Reg_AX); empty line is needed here. > + reg[RL78_SP_REGNUM] = pv_add_constant (reg[RL78_SP_REGNUM], -1); > + pv_area_store (stack, reg[RL78_SP_REGNUM], 1, reg[rsrc]); > + reg[RL78_SP_REGNUM] = pv_add_constant (reg[RL78_SP_REGNUM], -1); > + pv_area_store (stack, reg[RL78_SP_REGNUM], 1, reg[rsrc + 1]); > + after_last_frame_setup_insn = next_pc; > + } > + else if (opc.id == RLO_sub > + && opc.op[0].type == RL78_Operand_Register > + && opc.op[0].reg == RL78_Reg_SP > + && opc.op[1].type == RL78_Operand_Immediate) > + { > + int addend = opc.op[1].addend; and here. > + reg[RL78_SP_REGNUM] = pv_add_constant (reg[RL78_SP_REGNUM], -addend); > + after_last_frame_setup_insn = next_pc; > + } > + else > + { > + /* Terminate the prologue scan. */ > + break; > + } > + > + pc = next_pc; > + } > + > + /* Is the frame size (offset, really) a known constant? */ > + if (pv_is_register (reg[RL78_SP_REGNUM], RL78_SP_REGNUM)) > + result->frame_size = reg[RL78_SP_REGNUM].k; > + > + /* Record where all the registers were saved. */ > + pv_area_scan (stack, check_for_saved, (void *) result); > + > + result->prologue_end = after_last_frame_setup_insn; > + > + do_cleanups (back_to); > +} > + > + > +/* Implement the "pointer_to_address" gdbarch method. */ > +static CORE_ADDR > +rl78_pointer_to_address (struct gdbarch *gdbarch, > + struct type *type, const gdb_byte *buf) > +{ > + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > + CORE_ADDR addr > + = extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order); > + > + /* Is it a code address? */ > + if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_FUNC > + || TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_METHOD > + || TYPE_CODE_SPACE (TYPE_TARGET_TYPE (type)) > + || TYPE_LENGTH (type) == 4 If a pointer points to an integer (it is a data address, and size is 4-byte), we will get the instruction address, which is not correct, IMO. > + ) I think this parenthesis should be put in previous line, IMO. > + return rl78_make_instruction_address (addr); > + else > + return rl78_make_data_address (addr); > +} > + > + > +/* Implement the "unwind_pc" gdbarch method. */ > +static CORE_ADDR > +rl78_unwind_pc (struct gdbarch *arch, struct frame_info *next_frame) > +{ > + struct gdbarch_tdep *tdep = gdbarch_tdep (arch); `tdep' is not used. > + return rl78_addr_bits_remove > + (arch, frame_unwind_register_unsigned (next_frame, RL78_PC_REGNUM)); > +} > + > +/* Implement the "unwind_sp" gdbarch method. */ > +static CORE_ADDR > +rl78_unwind_sp (struct gdbarch *arch, struct frame_info *next_frame) > +{ > + struct gdbarch_tdep *tdep = gdbarch_tdep (arch); Likewise. -- Yao (齐尧)