From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70484 invoked by alias); 28 Feb 2017 11:39:55 -0000 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 Received: (qmail 70474 invoked by uid 89); 28 Feb 2017 11:39:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=rightly, pasted, autoconf, 2.64 X-HELO: mail-pg0-f49.google.com Received: from mail-pg0-f49.google.com (HELO mail-pg0-f49.google.com) (74.125.83.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 28 Feb 2017 11:39:50 +0000 Received: by mail-pg0-f49.google.com with SMTP id 25so4279091pgy.0 for ; Tue, 28 Feb 2017 03:39:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Bzp9Fv+WphrniG8cqbkgh7RQoSg4HEbKurNLm+aTiNI=; b=MifRWyRQ5oyoNm6nKgkCCKTCNf4T2Is9K7ImWY4b91Cy3Mj/gmDuY/JDwAMA5qPx/4 LpSz479CS7HiKD3QM9AjMD24edV9wJ5N647uy2mRs0sUxia6Xsva+HcVgyt42IlmSqZl RdkAQdRBSCb2ZPNUr3ta4v+hd4kTTxKwHUc6MrJScsWJQPaQ8SEdY4rmlRVB1gMRdjhr PAYsBY6gNb50Hfyizn5qj6SQPh4Iymq27UZGwziNHxUZkLA5Zdqm74KCRDEyil5//Bud HJ147alPuv6o/dSDvVEvNM1M3PSm9XX9KXtkt5YgGZ0LRBZNLomtwpWfyVBKLBrQaQFG M53w== X-Gm-Message-State: AMke39mejSP4As8m9id9vw59Xpa09egJ6VDcHj6xTrAveOH6uHblyIuNdl4kYAPjqtrshw== X-Received: by 10.84.129.2 with SMTP id 2mr2383238plb.119.1488280076682; Tue, 28 Feb 2017 03:07:56 -0800 (PST) Received: from localhost (z64.58-98-174.ppp.wakwak.ne.jp. [58.98.174.64]) by smtp.gmail.com with ESMTPSA id x65sm3578802pfx.115.2017.02.28.03.07.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 Feb 2017 03:07:55 -0800 (PST) Date: Tue, 28 Feb 2017 11:39:00 -0000 From: Stafford Horne To: gdb-patches@sourceware.org, openrisc@lists.librecores.org Subject: Re: [PATCH v2 4/6] sim: or1k: add or1k target to sim Message-ID: <20170228110753.GY2449@lianli.shorne-pla.net> References: <92c04bd768b66a48e03850af185e975d01605435.1484967575.git.shorne@gmail.com> <20170214185208.GJ28432@vapier> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170214185208.GJ28432@vapier> User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00714.txt.bz2 On Tue, Feb 14, 2017 at 01:52:08PM -0500, Mike Frysinger wrote: > On 21 Jan 2017 12:03, Stafford Horne wrote: > > --- /dev/null > > +++ b/sim/or1k/Makefile.in > > > > +# Extra headers included by sim-main.h. > > you shouldn't need to do any manual dep management. i'd suggest > deleting all this logic and seeing if things still work. Hi Mike, I have started to look into this. Its kind of interesting, what the original author was doing here is putting both 32 and 64 bit families in the same sim, something that I think is not currently supported by the cgen dep management. If I try to general all, I rightly get: ERROR: app requires all selected cpu families to have same word size Would you have a suggestion for this? - Should 32 and 64 bits be in separate sim_arch directories? - SHould we modify cgen to support handling different word sizes at the same time? Like the author has done here? At the moment I think I will work on getting the 32bit sim working alone. -Stafford > > --- /dev/null > > +++ b/sim/or1k/configure.ac > > @@ -0,0 +1,41 @@ > > +dnl Process this file with autoconf to produce a configure script. > > +AC_PREREQ(2.64)dnl > > +AC_INIT(Makefile.in) > > +sinclude(../common/acinclude.m4) > > + > > + case "${target_alias}" in > > drop the indentation > > > + or1k-linux*|or1knd-linux*) > > + traps_obj=traps32-linux.o > > + ;; > > + or1k-*|or1knd-*) > > + traps_obj=traps32.o > > + ;; > > + esac > > we really don't like to see this kind of logic. can't you do a single > build and then select the right details at runtime ? > > --- /dev/null > > +++ b/sim/or1k/or1k.h > > @@ -0,0 +1,38 @@ > > +#ifndef OR1K_H > > +#define OR1K_H > > all files should have a header with copyright & license info > > also, please use namespaced defines like SIM_OR1K_H > > > --- /dev/null > > +++ b/sim/or1k/sim-if.c > > @@ -0,0 +1,318 @@ > > +/* Main simulator entry points specific to the OR1K. > > + Copyright (C) 1996-1999, 2003, 2007-2012 Free Software Foundation, > > + Inc. > > + Contributed by Cygnus Support. > > looks like you need to double check where you copied & pasted things from. > cygnus isn't doing this port ;). > > > +SIM_DESC > > +sim_open (kind, callback, abfd, argv) > > + SIM_OPEN_KIND kind; > > + host_callback *callback; > > + struct bfd *abfd; > > + char * const *argv; > > you need to update all your prototypes. we don't do old style ones like > this anymore. > > > +#ifdef HAVE_DV_SOCKSER /* FIXME: was done differently before */ > > + if (dv_sockser_install (sd) != SIM_RC_OK) > > + { > > + free_state (sd); > > + return 0; > > + } > > +#endif > > delete this. common code handles it for you now. > > > + /* Allocate core managed memory if none specified by user. > > + Use address 4 here in case the user wanted address 0 unmapped. */ > > + if (sim_core_read_buffer (sd, NULL, read_map, &c, 4, 1) == 0) { > > + sim_do_commandf (sd, "memory region 0,0x%x", OR1K_DEFAULT_MEM_SIZE); > > + } > > this isn't using GNU style for the braces/if body. this comes up > multiple times in this patch, so please fix them all. > > > + /* check for/establish the reference program image */ > > this comment isn't using GNU style. this comes up > multiple times in this patch, so please fix them all. > > > + > > + for (c = 0; c < MAX_NR_PROCESSORS; ++c) > > + { > > + SIM_CPU *cpu = STATE_CPU (sd, i); > > + /* Only needed for profiling, but the structure member is small. */ > > + memset (CPU_OR1K_MISC_PROFILE (cpu), 0, > > + sizeof (* CPU_OR1K_MISC_PROFILE (cpu))); > > + > > +#ifdef WANT_CPU_OR1K32BF > > + or1k32bf_h_spr_set_raw(cpu, SPR_ADDR(SYS,VR), or1k_vr); > > + or1k32bf_h_spr_set_raw(cpu, SPR_ADDR(SYS,UPR), or1k_upr); > > + or1k32bf_h_spr_set_raw(cpu, SPR_ADDR(SYS,CPUCFGR), or1k_cpucfgr); > > + > > + or1k32bf_cpu_init (sd, cpu); > > +#endif > > + } > > this loop has to call these funcs: > CPU_REG_FETCH > CPU_REG_STORE > CPU_PC_FETCH > CPU_PC_STORE > > > + /* Store in a global so things like sparc32_dump_regs can be invoked > > + from the gdb command line. */ > > + current_state = sd; > > you must kill current_state. we do not allow global state anymore. > > > --- /dev/null > > +++ b/sim/or1k/sim-main.h > > > > +typedef IAI sim_cia; > > +#define CIA_GET(cpu) CPU_PC_GET (cpu) > > +#define CIA_SET(cpu,val) CPU_PC_SET ((cpu),(val)) > > these defines are dead -> delete > > > +typedef struct _sim_cpu SIM_CPU; > > delete this > > > +#ifdef OR1K_LINUX > > nothing defines this -> delete > > > +#if (WITH_SMP) > > +#define STATE_CPU(sd, n) ((sd)->cpu[n]) > > +#else > > +#define STATE_CPU(sd, n) ((sd)->cpu[0]) > > +#endif > > delete these -- common code does it for you now > -mike