Compile multiple files as one (.vm named after the first one) so we can inline itoa into a new fact.asm that shows both recursive and iterative factorial code (#42)

* Compile multiple files as one (.vm named after the first one) so we can inline itoa into a new fact.asm that shows both recursive and iterative factorial code

* Add a print util to not repeat the loadi

* minor lint and also exercize the cvm for fact. shows bug with incrs

* fix cvm to match loading accumulator content upon incrs

* abort on any file error

* less silly end of Compile

* Add a comment explaining the weird stack var if

Author: ldemailly

Makefile

@@ -52,6 +52,11 @@ grol_cvm: Makefile cvm/cvm.c cvm/cvm.h
 cvm-loop: grol_cvm
 	time ./grol_cvm programs/loop.vm
 
+fact: vm grol_cvm
+	./vm compile programs/fact.asm programs/itoa.asm
+	./vm run -quiet programs/fact.vm
+	./grol_cvm programs/fact.vm
+
 debug-cvm: Makefile cvm/cvm.c cvm/cvm.h
 	$(CC) -O3 -Wall -Wextra -pedantic -Werror -DDEBUG=1 -o grol_cvm cvm/cvm.c
 	./grol_cvm programs/simple.vm
@@ -80,7 +85,7 @@ install:
 	vm version
 
 
-test: vm unit-tests itoa-test
+test: vm unit-tests itoa-test fact
 
 unit-tests:
 	CGO_ENABLED=0 go test -tags $(GO_BUILD_TAGS) ./...
@@ -100,7 +105,7 @@ cpu/syscall_string.go: cpu/syscall.go
 	go generate ./cpu # if this fails go install golang.org/x/tools/cmd/stringer@latest
 
 .PHONY: all lint generate test clean run build install unit-tests
-.PHONY: show_cpu_profile show_mem_profile native debug-cvm
+.PHONY: show_cpu_profile show_mem_profile native debug-cvm fact
 
 show_cpu_profile:
 	-pkill pprof

asm/asm.go

@@ -24,33 +24,35 @@ type Line struct {
 }
 
 func Compile(files ...string) int {
-	for _, file := range files {
+	readers := make([]io.Reader, 0, len(files))
+	var writer *bufio.Writer
+	for i, file := range files {
 		log.Infof("Compiling file: %s", file)
 		f, err := os.Open(file)
 		if err != nil {
-			log.Errf("Failed to open file %s: %v", file, err)
-			continue
+			return log.FErrf("Failed to open file %s: %v", file, err)
 		}
 		defer f.Close()
 		// replace .asm with .vm
-		outputFile := strings.TrimSuffix(file, ".asm") + ".vm"
-		log.Infof("Output file: %s", outputFile)
-		out, err := os.Create(outputFile)
-		if err != nil {
-			log.Errf("Failed to create output file %s: %v", outputFile, err)
-			continue
+		if !strings.HasSuffix(file, ".asm") {
+			return log.FErrf("Invalid file extension for %s, expected .asm", file)
 		}
-		defer out.Close()
-		writer := bufio.NewWriter(out)
-		defer writer.Flush()
-		_, _ = writer.WriteString(cpu.HEADER)
-		reader := bufio.NewReader(f)
-		ret := compile(reader, writer)
-		if ret != 0 {
-			return ret
+		if i == 0 {
+			outputFile := strings.TrimSuffix(file, ".asm") + ".vm"
+			log.Infof("Output file: %s", outputFile)
+			out, err := os.Create(outputFile)
+			if err != nil {
+				return log.FErrf("Failed to create output file %s: %v", outputFile, err)
+			}
+			defer out.Close()
+			writer = bufio.NewWriter(out)
+			defer writer.Flush()
+			_, _ = writer.WriteString(cpu.HEADER)
 		}
+		readers = append(readers, f)
 	}
-	return 0
+	reader := bufio.NewReader(io.MultiReader(readers...))
+	return compile(reader, writer)
 }
 
 //nolint:gocyclo // it's a full parser.
@@ -284,8 +286,8 @@ func compile(reader *bufio.Reader, writer *bufio.Writer) int {
 			op = op.SetOpcode(cpu.Ret)
 			op = op.SetOperand(cpu.ImmediateData(returnN))
 			log.Debugf("Return -> Ret %d", returnN)
-			returnN = 0
-			clear(varmap)
+			// Don't reset returnN or varmap because there could be more than 1 return
+			// point.
 		default:
 			instrEnum, ok := cpu.InstructionFromString(instr)
 			if !ok {
@@ -294,9 +296,15 @@ func compile(reader *bufio.Reader, writer *bufio.Writer) int {
 			log.Debugf("Parsing instruction: %s %v", instrEnum, args)
 			if instrEnum >= cpu.LoadS { // for stack instructions, resolve var references
 				for i, v := range args {
+					if !isAddressLabel(v) {
+						continue
+					}
 					if idx, ok := varmap[v]; ok {
 						log.Debugf("Resolved var %s to index %d", v, idx)
 						args[i] = fmt.Sprintf("%d", idx)
+					} else if instrEnum != cpu.SysS || i != 0 {
+						// First argument of SysS is the syscall name not a stack variable.
+						return log.FErrf("Unknown stack variable: %s", v)
 					}
 				}
 			}

cpu/cpu.go

@@ -348,7 +348,7 @@ func execute(pc ImmediateData, program []Operation, accumulator int64) (int64, i
 			pc = ImmediateData(stack[stackPtr])
 			stackPtr--
 			if Debug {
-				log.Debugf("Return   at PC: %d, returning to PC: %d - SP = %d %v", oldPC, pc, stackPtr, stack[:stackPtr+1])
+				log.Debugf("Return  at PC: %d, returning to PC: %d - SP = %d %v", oldPC, pc, stackPtr, stack[:stackPtr+1])
 			}
 			continue
 		case Push:
@@ -415,10 +415,12 @@ func execute(pc ImmediateData, program []Operation, accumulator int64) (int64, i
 			arg := op.Operand()
 			offset := int(arg >> 8)
 			value := int8(arg & 0xff) //nolint:gosec // 0xff implies can't overflow (and we want the sign bit too)
-			stack[stackPtr-offset] += Operation(value)
+			oldValue := stack[stackPtr-offset]
+			accumulator = int64(oldValue) + int64(value)
+			stack[stackPtr-offset] = Operation(accumulator)
 			if Debug {
 				log.Debugf("IncrS   at PC: %d, offset: %d, value: %d -> %d - SP = %d %v",
-					pc, offset, value, stack[stackPtr-offset], stackPtr, stack[:stackPtr+1])
+					pc, offset, value, accumulator, stackPtr, stack[:stackPtr+1])
 			}
 		case IdivS:
 			offset := int(op.Operand())

cvm/cvm.c

@@ -331,7 +331,8 @@ void run_program(CPU *cpu) {
       int8_t value = (int8_t)(arg & 0xFF);
       DEBUG_PRINT("IncrS  at PC %" PRId64 ", offset %d, by %d, SP=%d\n",
                   cpu->pc, offset, value, stack_ptr);
-      stack[stack_ptr - offset] += (Operation)value;
+      cpu->accumulator = (int64_t)stack[stack_ptr - offset] + (int64_t)value;
+      stack[stack_ptr - offset] = (Operation)cpu->accumulator;
       DEBUG_PRINT("IncrS  new value %" PRId64 "\n",
                   (int64_t)stack[stack_ptr - offset]);
     } break;

programs/fact.asm

@@ -0,0 +1,55 @@
+; factorial
+; depends on itoa, so compile with
+; vm compile programs/fact.asm programs/itoa.asm
+
+    sys write fact_rec_str
+    loadI 5
+    call print
+    call factrec
+    call itoa
+    sys write fact_iter_str
+    loadI 7
+    call print
+    call facti
+    call itoa
+    sys exit 0
+
+factrec: ; recursive factorial
+    var n
+    subi 1
+    jnz more
+    loadI 1
+    return
+more:
+    call factrec
+    muls n
+    return
+
+facti: ; iterative factorial
+    var result n
+    subi 1
+    storeS n
+  loop:
+    muls result
+    storeS result
+    incrs -1 n
+    jnz loop
+end:
+    loadS result
+    return
+
+; print accumulator and put its value back (instead of the bytes written returned by itoa)
+print:
+    var acc
+    sys write fact_str
+    loadS acc
+    call itoa
+    loadS acc
+    return
+
+fact_rec_str:
+    str8 "Recursive "
+fact_iter_str:
+    str8 "Iterative "
+fact_str:
+    str8 "Factorial of "