Merge pull request #81 from kulp/collected-fixes

Address collected minor issues

Makefile

@@ -56,11 +56,11 @@ distclean:: clobber
 clean clobber::
 	-rmdir $(BUILDDIR)/devices $(BUILDDIR) build # fail, ignore if non-empty
 	-$(MAKE) -C $(TOP)/test $@
+	-$(MAKE) -C $(TOP)/ex $@
 	-$(MAKE) -C $(TOP)/hw/icarus $@
 
 clobber_FILES += $(BUILDDIR)/*.gc??
 clobber_FILES += $(BUILDDIR)/coverage.info*
-clobber_FILES += $(BUILDDIR)/PERIODS.mk
 clobber_FILES += $(BUILDDIR)/coverage_html_*
 clobber_FILES += $(BUILDDIR)/vpidevices.vpi
 clobber_FILES += $(TOP)/test/op/*.texe

hw/icarus/Makefile

@@ -2,7 +2,7 @@ TOP := ../..
 include $(TOP)/mk/common.mk
 include $(TOP)/mk/rules.mk
 
-PERIODS_DEFAULT ?= 32
+PERIODS_DEFAULT ?= 1000000 # One million cycles should be enough for anybody.
 
 CFLAGS += -Wall -Wextra -Wshadow -pedantic-errors -std=c99 $(VPI_CFLAGS)
 CFLAGS += -I$(TOP)/src
@@ -47,7 +47,7 @@ vpath %.texe $(TOP)/ex $(TOP)/test/run/sdl
 vpath %.tas $(TOP)/ex $(TOP)/test/run/sdl
 
 LOGFILE = Top.vcd
-run_%: PLUSARGS += +PERIODS=$(firstword $(PERIODS) $(PERIODS_$*) $(PERIODS_DEFAULT)) \
+run_%: PLUSARGS += +PERIODS=$(PERIODS_DEFAULT) \
                    +LOGFILE=$(LOGFILE) \
                    $(PLUSARGS_EXTRA) \
                    #

hw/verilog/sim/simtop.v

@@ -20,11 +20,28 @@ module Top();
 
     Tenyr #(.LOADFILE(LOADFILE)) tenyr(.clk, .reset, .halt);
 
+    task end_simulation;
+    begin
+        integer row, col, i;
+        if ($test$plusargs("DUMPENDSTATE")) begin
+            for (row = 0; row < 3; row = row + 1) begin
+                $write("state: ");
+                for (col = 0; col < 6 && row * 6 + col < 16; col = col + 1) begin
+                    i = row * 6 + col;
+                    $write("%c %08x ", 65 + i, tenyr.core.regs.store[i]);
+                end
+                $write("\n");
+            end
+        end
+        $finish;
+    end
+    endtask
+
 `ifdef __ICARUS__
     // TODO The `ifdef guard should really be controlling for VPI availability
-    reg [800:0] filename;
-    reg [800:0] logfile = "Top.vcd";
-    integer periods = 64;
+    reg [8 * 4096:0] filename;
+    reg [8 * 4096:0] logfile = "Top.vcd";
+    integer periods = 32'hffffffff;
     integer clk_count = 0;
     integer insn_count = 0;
     integer temp;
@@ -43,30 +60,18 @@ module Top();
             logfile = filename;
         $dumpfile(logfile);
         $dumpvars;
-        #(periods * `CLOCKPERIOD) begin:ending
-            integer row, col, i;
-            if ($test$plusargs("DUMPENDSTATE")) begin
-                for (row = 0; row < 3; row = row + 1) begin
-                    $write("state: ");
-                    for (col = 0; col < 6 && row * 6 + col < 16; col = col + 1) begin
-                        i = row * 6 + col;
-                        $write("%c %08x ", 65 + i, tenyr.core.regs.store[i]);
-                    end
-                    $write("\n");
-                end
-            end
-            $finish;
-        end
+        #(periods * `CLOCKPERIOD) end_simulation();
     end
+`endif
 
     always #`CLOCKPERIOD begin
+        if (tenyr.core.insn == 32'hffffffff)
+            end_simulation();
         clk_count = clk_count + 1;
         if (tenyr.core.state == tenyr.core.s3) begin
             insn_count = insn_count + 1;
         end
     end
 
-`endif
-
 endmodule
 

mk/misc.mk

@@ -174,7 +174,6 @@ check_behaviour_tas: check_behaviour_%: %$(EXE_SUFFIX)
 	(! $($*) -d -f memh $(MEMHD)backward.memh &>/dev/null )                 && $(MAKESTEP) "    ... validated memh lack of backward support ok"
 	$($*) -d $(OBJD)bad_version.to 2>&1 | $(GREP) -i "unhandled version"    && $(MAKESTEP) "    ... unhandled version ok"
 	$($*) -d $(OBJD)toolarge.to 2>&1 | $(GREP) "too large"                  && $(MAKESTEP) "    ... too-large ok"
-	$($*) -d $(OBJD)toolarge2.to 2>&1 | $(GREP) "too large"                 && $(MAKESTEP) "    ... too-large 2 ok"
 	$($*) -d $(OBJD)too-many-symbols.to 2>&1 | $(GREP) "too large"          && $(MAKESTEP) "    ... too many symbols ok"
 	$($*) -d $(OBJD)too-many-relocs.to 2>&1 | $(GREP) "too large"           && $(MAKESTEP) "    ... too many relocs ok"
 	$($*) -d $(OBJD)overlong-symbol.to 2>&1 | $(GREP) "too large"           && $(MAKESTEP) "    ... overlong symbol ok"
@@ -186,10 +185,8 @@ check_behaviour_tld: OBJD = $(TOP)/test/misc/obj/
 check_behaviour_tld: check_behaviour_%: %$(EXE_SUFFIX)
 	@$(MAKESTEP) "Checking $* behaviour ... "
 	$($*) /dev/null 2>&1 | $(GREP) -i "end of file"                             && $(MAKESTEP) "    ... too-small ok"
-	$($*) $(OBJD)toolarge.to 2>&1 | $(GREP) "too large"                         && $(MAKESTEP) "    ... too-large ok"
 	$($*) -o $(OBJD) /dev/null 2>&1 | $(GREP) -i "failed to open"               && $(MAKESTEP) "    ... failed to open ok"
 	$($*) $(OBJD)duplicate.to $(OBJD)duplicate.to 2>&1 | $(GREP) -i "duplicate" && $(MAKESTEP) "    ... duplicate symbols ok"
-	$($*) $(OBJD)zerorecs.to 2>&1 | $(GREP) -i "has no records"                 && $(MAKESTEP) "    ... zero records ok"
 	$($*) $(OBJD)tworecs.to 2>&1 | $(GREP) -i "more than one record"            && $(MAKESTEP) "    ... multiple records ok"
 	(! $($*) $(OBJD)too-many-records.to &>/dev/null )                           && $(MAKESTEP) "    ... too many records ok"
 	$($*) $(OBJD)unresolved.to 2>&1 | $(GREP) -i "missing definition"           && $(MAKESTEP) "    ... unresolved ok"
@@ -301,9 +298,9 @@ check_sim::
 check_sim_flavour: check_sim_demo check_sim_op check_sim_run
 
 check_hw_icarus_demo check_hw_icarus_op check_hw_icarus_run: export context=hw_icarus
-check_hw_icarus_demo check_hw_icarus_op check_hw_icarus_run: check_hw_icarus_pre PERIODS.mk
+check_hw_icarus_demo check_hw_icarus_op check_hw_icarus_run: check_hw_icarus_pre
 
-check_hw_icarus_demo: export run=$(MAKE) --no-print-directory -s -C $(TOP)/hw/icarus -f $(abspath $(BUILDDIR))/PERIODS.mk -f Makefile BUILDDIR=$(abspath $(BUILDDIR)) run_$*_demo | grep -v -e ^WARNING: -e ^ERROR: -e ^VCD
+check_hw_icarus_demo: export run=$(MAKE) --no-print-directory -s -C $(TOP)/hw/icarus BUILDDIR=$(abspath $(BUILDDIR)) run_$*_demo | grep -v -e ^WARNING: -e ^ERROR: -e ^VCD
 check_sim_demo: export run=$(tsim) $(tsim_FLAGS) $(TOP)/ex/$*_demo.texe
 
 # "SDL-related" tests
@@ -323,7 +320,6 @@ $(TOP)/ex/%.texe: ; $(MAKE) -C $(@D) $(@F)
 $(TOP)/test/run/sdl/%.texe: ; $(MAKE) -C $(TOP)/test run/sdl/$(@F)
 
 export SDL_VIDEODRIVER=dummy
-PERIODS.mk: $(SDL_RUNS:%=PERIODS_%.mk)
 check_hw_icarus_run: $(SDL_RUNS:%=test_run_%)
 
 # Allow testing for a nonexistent file.
@@ -342,28 +338,10 @@ check_sim_demo check_hw_icarus_demo: $(DEMOS:%=test_demo_%)
 check_sim_op   check_hw_icarus_op:   $(OPS:%=  test_op_%  )
 check_sim_run  check_hw_icarus_run:  $(RUNS:%= test_run_% )
 
-# TODO make op tests take a fixed or predictable maximum amount of time
-# The number of cycles necessary to run a code in Verilog simulation is
-# determined by running it in tsim and multiplying the number of instructions
-# executed by the number of cycles per instruction (currently 10). A margin of
-# 2x is added to allow testcases not always to take exactly the same number of
-# instructions to complete.
 vpath %.texe $(TOP)/test/op $(TOP)/ex $(TOP)/test/run
-clean_FILES += $(BUILDDIR)/PERIODS_*.mk
-PERIODS_%.mk: %.texe $(build_tsim)
-	@$(MAKESTEP) -n "Computing cycle count for '$*' ... "
-	$(ECHO) -n PERIODS_$*= > $@
-	echo $$(($$($(tsim) -d $< 2>&1 | sed -En '/^.*executed: ([0-9]+)/{s//\1/;p;}') * 20)) >> $@
-	cp -f $(TOP)/mk/$(@F) $@ 2>/dev/null || true # override with forced version if existing
-	@$(MAKESTEP) ok
-
-vpath PERIODS_%.mk $(TOP)/mk
-PERIODS.mk: $(patsubst %,PERIODS_%.mk,$(OPS) $(DEMOS:%=%_demo) $(RUNS))
-	cat $^ > $@
 
 check_sim_op check_sim_run: export run=$(tsim) $(tsim_FLAGS) -vvvv $(texe) 2>&1 | grep -o 'B.[[:xdigit:]]\{8\}' | tail -n1 | grep -q 'f\{8\}'
-check_hw_icarus_op: PERIODS.mk
-check_hw_icarus_op check_hw_icarus_run: export run=$(MAKE) -s --no-print-directory -C $(TOP)/hw/icarus -f $(abspath $(BUILDDIR))/PERIODS.mk -f Makefile run_$* VPATH=$(TOP)/test/op:$(TOP)/test/run BUILDDIR=$(abspath $(BUILDDIR)) PLUSARGS_EXTRA=+DUMPENDSTATE | grep -v -e ^WARNING: -e ^ERROR: -e ^VCD | grep -o 'B.[[:xdigit:]]\{8\}' | tail -n1 | grep -q 'f\{8\}'
+check_hw_icarus_op check_hw_icarus_run: export run=$(MAKE) -s --no-print-directory -C $(TOP)/hw/icarus run_$* VPATH=$(TOP)/test/op:$(TOP)/test/run BUILDDIR=$(abspath $(BUILDDIR)) PLUSARGS_EXTRA=+DUMPENDSTATE | grep -v -e ^WARNING: -e ^ERROR: -e ^VCD | grep -o 'B.[[:xdigit:]]\{8\}' | tail -n1 | grep -q 'f\{8\}'
 
 check_compile: $(build_tas) $(build_tld)
 	@$(MAKESTEP) "Building tests from test/ ..."

src/asmif.c

@@ -255,17 +255,15 @@ static int fixup_deferred_exprs(struct parse_data *pd)
     return rc;
 }
 
-static int mark_globals(struct symbol_list *symbols, struct global_list *globals)
+static void mark_globals(struct symbol_list *symbols, struct global_list *globals)
 {
     struct symbol *which;
     list_foreach(global_list, g, globals)
         if ((which = symbol_find(symbols, g->name)))
             which->global = 1;
         else
-            debug(0, "Symbol `%s' was marked as a global, but was not defined",
+            fatal(0, "Symbol `%s' was marked as a global, but was not defined",
                     g->name);
-
-    return 0;
 }
 
 static int check_symbols(struct symbol_list *symbols)

src/tld.c

@@ -125,13 +125,13 @@ static void do_link_build_state(struct link_state *s, void **objtree, void **def
         assert(i->rec_count < OBJ_MAX_REC_CNT);
 
         if (i->rec_count == 0) {
-            debug(0, "Object has no records, skipping");
+            debug(1, "Object has no records, skipping");
             continue;
         }
 
         // TODO support more than one record per object
         if (i->rec_count > 1)
-            debug(0, "Object has more than one record, only using first");
+            fatal(0, "Object has more than one record (unsupported)");
 
         struct objmeta *meta = calloc(1, sizeof *meta);
         meta->state = s;
@@ -216,7 +216,7 @@ static void do_link_relocate(struct obj_list *ol, void **objtree, void **defns)
 {
     list_foreach(obj_list, Node, ol) {
         if (!Node->obj->rec_count) {
-            debug(0, "Object has no records, skipping");
+            debug(1, "Object has no records, skipping");
             continue;
         }
 

src/tsim.c

@@ -513,13 +513,13 @@ int main(int argc, char *argv[])
     void *run_ud = NULL;
     rc = s->run_sim(s, &ops, &run_ud, NULL);
     if (rc < 0)
-        fprintf(stderr, "Error during simulation, P=0x%08x\n", s->machine.regs[15]);
+        fatal(0, "Error during simulation, P=0x%08x", s->machine.regs[15]);
 
     fclose(infile);
 
     rc = devices_teardown(s);
     if (rc != 0)
-        fprintf(stderr, "Error during device teardown\n");
+        fatal(0, "Error during device teardown");
 
     if (s->conf.debugging > 0)
         fprintf(stderr, "Instructions executed: %lu\n", s->insns_executed);