2
\$\begingroup\$

I'm studying C with a Deitel's book. The book asks me to write a program that simulates a virtual PC (with virtual memory in an array called memory[100]). Then the book asks me to write a program that sums 10 numbers. I did it but I had to initialize some variables (memory[20], memory[21], memory[22]) before running the program. There is any way to make this program without initially storing the variables and using the same number of instructions?

These are valid instructions:

#define READ 10;
#define WRITE 11;
#define LOAD 20;
#define STORE 21;
#define ADD 30;
#define SUBTRACT 31;
#define DIVIDE 32;
#define MULTIPLY 33;
#define BRANCH 40;
#define BRANCHANG 41;
#define BRANCHZERO 42;
#define HALT 43;

And the code:

int main(){
int memory[100] = {2022,1022,3022,2122,2021,3120,2121,4209,4000,1122,4300}; // my set of instruction for sum 10 numbers
memory[99] = -99999;
memory[20] = 1; // the variables initializations that i want to remove
memory[21] = 10;
memory[22] = 0;
int accumulator = 0000;
int instCounter = 00;
int instRegistrer = 0000;
int operationCode = 00;
int operand = 00;


while(memory[instCounter] >= 0){
    printf("\n**%d**  ", instCounter);
    instRegistrer = memory[instCounter];
    operationCode = instRegistrer / 100;
    operand = instRegistrer % 100;
    switch(operationCode){
        case READ:
            printf("READ ");
            scanf("%d", &memory[operand]);
            instCounter++;
            break;
        case WRITE:
            printf("WRITE ");
            printf("%d", memory[operand]);
            instCounter++;
            break;
        case LOAD:
            printf("LOAD ");
            accumulator = memory[operand];
            instCounter++;
            break;
        case STORE:
            printf("STORE ");
            memory[operand] = accumulator;
            instCounter++;
            break;
        case ADD:
            printf("ADD ");
            accumulator += memory[operand];
            instCounter++;
            break;
        case SUBTRACT:
            printf("SUBTRACT ");
            accumulator -= memory[operand];
            instCounter++;
            break;
        case DIVIDE:
            printf("DIVIDE ");
            accumulator /= memory[operand];
            instCounter++;
            break;
        case MULTIPLY:
            printf("MULTIPLY ");
            accumulator *= memory[operand];
            instCounter++;
            break;
        case BRANCH:
            printf("BRANCH ");
            instCounter = operand;
            break;
        case BRANCHANG:
            printf("BRANCHANG ");
            if(accumulator < 0)instCounter = operand;
            else instCounter++;
            break;
        case BRANCHZERO:
            printf("BRANCHZERO ");
            if(accumulator == 0)instCounter = operand;
            else instCounter++;
            break;
        case HALT:
            printf("HALT ");
            instCounter = 99;
            break;
    }
}
return 0;
}
\$\endgroup\$

closed as off-topic by 200_success, Vogel612 Dec 25 '18 at 16:01

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – 200_success, Vogel612
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ @1201ProgramAlarm Can you illuminate us? What's the error? \$\endgroup\$ – Reinderien Dec 24 '18 at 18:05
  • 4
    \$\begingroup\$ @Reinderien As you mention in your answer, all those semicolons in the #define macros, which will expand to case 10;: etc. which won't compile. \$\endgroup\$ – 1201ProgramAlarm Dec 25 '18 at 0:42
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Feel free to post a follow-up question linking back to this one instead. \$\endgroup\$ – Mast Dec 25 '18 at 7:42
  • 3
    \$\begingroup\$ Closing this question as broken, since the code does not compile. Please refrain from changing the code after receiving answers as outlined in the policy above. \$\endgroup\$ – Vogel612 Dec 25 '18 at 16:03
  • 2
    \$\begingroup\$ Note that this answer is currently being discussed on meta \$\endgroup\$ – Vogel612 Dec 25 '18 at 23:20
10
\$\begingroup\$

Your instruction list:

#define READ 10;
#define WRITE 11;
#define LOAD 20;
#define STORE 21;
#define ADD 30;
#define SUBTRACT 31;
#define DIVIDE 32;
#define MULTIPLY 33;
#define BRANCH 40;
#define BRANCHANG 41;
#define BRANCHZERO 42;
#define HALT 43;

has a few issues. First of all, you shouldn't add semicolons after those defines. Also, this would be better represented as an enum, since op codes are mutually exclusive.

Your memory initialization:

int memory[100] = {2022,1022,3022,2122,2021,3120,2121,4209,4000,1122,4300}; // my set of instruction for sum 10 numbers

also needs a few adjustments. Since none of these values will exceed 9999, you should be storing them as int16_t, not int. Furthermore, you shouldn't be writing raw machine code in that array. You should be constructing those values like

{
    100*LOAD + 22,
    100*READ + 22,
    // ...
}

Also, it seems like you store -99999 as a magic value to indicate program termination, when this is technically not necessary. I don't have the specifications for your VM, but it would make more sense to

  • have all of the memory contents be of type int16_t
  • on HALT, simply terminate the program

rather than

  • on HALT, jump to the end of memory
  • at the end of memory, have a magic negative value.

There's a typo in instRegistrer. It's "register", not "registrer".

I propose that, instead of having to manually write a different printf string for each instruction,

  1. declare an array of 44 character pointers
  2. define a macro that indexes into the array based on the opcode value, and initializes that element to a stringized (#) version of the opcode
  3. call the macro for every opcode that you have

You could take this strategy further and store a struct at each element of the array, containing a string name and an operation function pointer. This would nicely generalize your code and make execution of long programs faster.

You're also keen on avoiding separate assignment of memory contents for use by variables. This can be done by shifting your "data" memory to the address right after your "program" memory, and then doing the initialization in your array literal.

The following incorporates all of the above. It seems you're still learning basic C, so some of these concepts may require explanation - please let me know in the comments.

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

typedef enum
{
    READ = 10,
    WRIT = 11,
    LOAD = 20,
    STOR = 21,
    ADD = 30,
    SUB = 31,
    DIV = 32,
    MUL = 33,
    BRA = 40,
    BRNG = 41,
    BRZR = 42,
    HALT = 43
} OpCode;

typedef struct
{
    int16_t mem[100], acc;
    uint8_t counter;
} VM;

typedef struct
{
    const char *name;
    void (*execute)(VM *vm, uint8_t addr);
} Operation;

static uint16_t makeOp(OpCode code, uint8_t addr)
{
    return 100*code + addr;
}

#define DEFOP(op) ops[op] = (Operation){.name=#op, .execute=exc_##op}

static void exc_READ(VM *vm, uint8_t addr) {
    int32_t x;
    if (scanf("%d", &x) != 1)
    {
        perror("Failed to read integer");
        exit(1);
    }
    if (x >= 10000 || x <= -10000)
    {
        fprintf(stderr, "Invalid value %d", x);
        exit(1);
    }
    vm->mem[addr] = x;
    vm->counter++;
}
static void exc_WRIT(VM *vm, uint8_t addr) {
    printf("%d\n", vm->mem[addr]);
    vm->counter++;
}
static void exc_LOAD(VM *vm, uint8_t addr) {
    vm->acc = vm->mem[addr];
    vm->counter++;
}
static void exc_STOR(VM *vm, uint8_t addr) {
    vm->mem[addr] = vm->acc;
    vm->counter++;
}
static void exc_ADD(VM *vm, uint8_t addr) {
    vm->acc += vm->mem[addr];
    vm->counter++;
}
static void exc_SUB(VM *vm, uint8_t addr) {
    vm->acc -= vm->mem[addr];
    vm->counter++;
}
static void exc_DIV(VM *vm, uint8_t addr) {
    vm->acc /= vm->mem[addr];
    vm->counter++;
}
static void exc_MUL(VM *vm, uint8_t addr) {
    vm->acc *= vm->mem[addr];
    vm->counter++;
}
static void exc_BRA(VM *vm, uint8_t addr) {
    vm->counter = addr;
}
static void exc_BRNG(VM *vm, uint8_t addr) {
    if (vm->acc < 0)
        vm->counter = addr;
    else
        vm->counter++;
}
static void exc_BRZR(VM *vm, uint8_t addr) {
    if (!vm->acc)
        vm->counter = addr;
    else
        vm->counter++;
}
static void exc_HALT(VM *vm, uint8_t addr) {
    exit(0);
}

int main()
{
    const uint8_t X = 13, I = 12, DELTA = 11;

    Operation ops[HALT+1];
    DEFOP(READ);
    DEFOP(WRIT);
    DEFOP(LOAD);
    DEFOP(STOR);
    DEFOP(ADD);
    DEFOP(SUB);
    DEFOP(DIV);
    DEFOP(MUL);
    DEFOP(BRA);
    DEFOP(BRNG);
    DEFOP(BRZR);
    DEFOP(HALT);

    VM vm = {.counter = 0,
             .acc = 0,
             .mem = {
                makeOp(LOAD, X), // 0: beginning
                makeOp(READ, X),
                makeOp( ADD, X),
                makeOp(STOR, X),
                makeOp(LOAD, I),
                makeOp( SUB, DELTA),
                makeOp(STOR, I),
                makeOp(BRZR, 9),
                makeOp( BRA, 0),
                makeOp(WRIT, X), // 9: exit loop
                makeOp(HALT, 0),
                1,  // 11: DELTA
                10, // 12: I
                0   // 13: X
             }};

    for (;;) {
        int16_t current = vm.mem[vm.counter];
        uint8_t code = current/100,
                addr = current%100;
        const Operation *op = ops + code;
        printf("%02u: %4s %02u\n", vm.counter, op->name, addr);
        op->execute(&vm, addr);
    }
}
\$\endgroup\$
  • \$\begingroup\$ In the original code there was no semicolon after definies. I'm studying C and i am actually on the pointer chapter, i dont know what enum is. uint16_t is a good idea but It Is only an exercise, i dont mind about efficence. The instruction must be an integer of 4 digits. My question was there Is any way to sum 10 numbers without initialize variables in Memory. \$\endgroup\$ – Alex Dec 24 '18 at 18:32
  • 1
    \$\begingroup\$ An enum is basically an integer, but its values are constrained to a certain named list. Be careful about your 4-digit notation, because it sometimes doesn't do what you expect: for instance, 0244 is interpreted as octal instead of decimal. \$\endgroup\$ – Reinderien Dec 24 '18 at 19:40
  • \$\begingroup\$ There is a way to simplify your memory variable initialization. If you're allowed to use different addresses than 20/21/22, then you can include them at the end of the literal array initializer (but you'll have to adjust your addresses). \$\endgroup\$ – Reinderien Dec 24 '18 at 19:41
  • 5
    \$\begingroup\$ It seems like you have some reading to do :) \$\endgroup\$ – Reinderien Dec 24 '18 at 19:54
  • 5
    \$\begingroup\$ In the future, please refrain from answering off-topic questions. The code in the question was broken (did not compile, so couldn't have produced the correct result). Instead, flag/vote to close the question instead so it can be fixed by the author (and afterwards be reopened/reviewed). Thank you for your contributions! \$\endgroup\$ – Mast Dec 25 '18 at 7:46

Not the answer you're looking for? Browse other questions tagged or ask your own question.