Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor architecture specific configurations #47

Closed
jserv opened this issue Feb 4, 2021 · 2 comments
Closed

Refactor architecture specific configurations #47

jserv opened this issue Feb 4, 2021 · 2 comments
Assignees

Comments

@jserv
Copy link
Collaborator

jserv commented Feb 4, 2021

Instead of #43, I would propose an elegant way to specify the architecture and its configurations. The WIP changes are shown as following:

diff --git a/Makefile b/Makefile
index 9d3e9ca..8a31660 100644
--- a/Makefile
+++ b/Makefile
@@ -20,21 +20,22 @@ OBJS := $(SRCS:%.c=$(OUT)/%.o)
 deps := $(OBJS:%.o=%.o.d)
 TESTS := $(wildcard tests/*.c)
 TESTBINS := $(TESTS:%.c=$(OUT)/%.elf)
-TARGET_EXEC := `cat $(OUT)/target`
 
 all: config bootstrap
 
-config:
-ifeq (riscv,$(ARCH))
-	@$(VECHO) "$(RISCV_EXEC)" > $(OUT)/target
-	@$(VECHO) "#define TARGET_RISCV 1" > $@
-	@ln -s $(PWD)/$(SRCDIR)/riscv-codegen.c $(SRCDIR)/codegen.c
+# set ARM by default
+ifeq ($(strip $(ARCH)),riscv)
+ARCH = riscv
 else
-	@$(VECHO) "$(ARM_EXEC)" > $(OUT)/target
-	@$(VECHO) "#define TARGET_ARM 1" > $@
-	@ln -s $(PWD)/$(SRCDIR)/arm-codegen.c $(SRCDIR)/codegen.c
+ARCH = arm
 endif
-	@$(VECHO) "Target machine code switch to %s\n" "$$(cat out/target | sed 's/.*qemu-\([^ ]*\).*/\1/')"
+
+TARGET_EXEC = $($(shell echo $(ARCH) | tr a-z A-Z)_EXEC)
+
+config:
+	$(Q)ln -s $(PWD)/$(SRCDIR)/$(ARCH)-codegen.c $(SRCDIR)/codegen.c
+	$(call $(ARCH)-specific-defs) > $@
+	$(VECHO) "Target machine code switch to %s\n" $(ARCH)
 
 $(OUT)/tests/%.elf: tests/%.c $(OUT)/$(STAGE0)
 	$(VECHO) "  SHECC\t$@\n"
diff --git a/mk/arm.mk b/mk/arm.mk
index 0195288..5dbddd3 100644
--- a/mk/arm.mk
+++ b/mk/arm.mk
@@ -6,3 +6,10 @@ ARM_EXEC = echo WARN: unable to run
 endif
 
 export ARM_EXEC
+
+arm-specific-defs = \
+    $(Q)$(PRINTF) \
+" \#define ARCH_PREDEFIND \"__arm__\" /* defined by GNU C and RealView */\n\
+  \#define ELF_MACHINE 0x28 /* up to ARMv7/Aarch32 */\n \
+  \#define ELF_FLAGS 0x5000200\n \
+"
diff --git a/mk/riscv.mk b/mk/riscv.mk
index 19fe194..7c0fb62 100644
--- a/mk/riscv.mk
+++ b/mk/riscv.mk
@@ -5,4 +5,11 @@ $(warning "no qemu-riscv32 found. Please check package installation")
 RISCV_EXEC = echo WARN: unable to run
 endif
 
-export RISCV_EXEC
\ No newline at end of file
+export RISCV_EXEC
+
+riscv-specific-defs = \
+    $(Q)$(PRINTF) \
+" \#define ARCH_PREDEFIND \"__riscv\" /* Older versions of the GCC toolchain defined __riscv__ */\n\
+  \#define ELF_MACHINE 0xf3\n\
+  \#define ELF_FLAGS 0\n\
+"
diff --git a/src/cfront.c b/src/cfront.c
index af5f8d6..6da5206 100644
--- a/src/cfront.c
+++ b/src/cfront.c
@@ -2145,15 +2145,8 @@ void parse_internal()
     add_block(NULL, NULL);    /* global block */
     elf_add_symbol("", 0, 0); /* undef symbol */
 
-/* architecture defines */
-/* FIXME: use #ifdef ... #else ... #endif */
-#ifdef TARGET_ARM
-    add_alias("__arm__", "1"); /* defined by GNU C and RealView */
-#endif
-#ifdef TARGET_RISCV
-    /* Older versions of the GCC toolchain defined __riscv__ */
-    add_alias("__riscv", "1");
-#endif
+    /* architecture defines */
+    add_alias(ARCH_PREDEFIND, "1");
 
     /* binary entry point: read params, call main, exit */
     ii = add_instr(OP_label);
diff --git a/src/elf.c b/src/elf.c
index 27bfa12..e266aed 100644
--- a/src/elf.c
+++ b/src/elf.c
@@ -82,13 +82,7 @@ void elf_generate_header()
     elf_write_header_int(0);          /* EI_PAD: unused */
     elf_write_header_byte(2);         /* ET_EXEC */
     elf_write_header_byte(0);
-/* FIXME: use #ifdef ... #else ... #endif */
-#ifdef TARGET_ARM
-    elf_write_header_byte(0x28); /* ARM (up to ARMv7/Aarch32) */
-#endif
-#ifdef TARGET_RISCV
-    elf_write_header_byte(0xf3); /* RISC-V */
-#endif
+    elf_write_header_byte(ELF_MACHINE);
     elf_write_header_byte(0);
     elf_write_header_int(1);                          /* ELF version */
     elf_write_header_int(ELF_START + elf_header_len); /* entry point */
@@ -96,14 +90,7 @@ void elf_generate_header()
     elf_write_header_int(elf_header_len + elf_code_idx + elf_data_idx + 39 +
                          elf_symtab_index +
                          elf_strtab_index); /* section header offset */
-/* flags */
-/* FIXME: use #ifdef ... #else ... #endif */
-#ifdef TARGET_ARM
-    elf_write_header_int(0x5000200); /* ARM */
-#endif
-#ifdef TARGET_RISCV
-    elf_write_header_int(0);
-#endif
+    elf_write_header_int(ELF_FLAGS);
     elf_write_header_byte(0x34); /* header size */
     elf_write_header_byte(0);
     elf_write_header_byte(0x20); /* program header size */

Warning: the patch did not work yet, but it should be fairly expressive.

@eecheng87
Copy link
Collaborator

eecheng87 commented Feb 7, 2021

Because the lifetime of $(ARCH), we can't use testing command described in README:

make ARCH=riscv # bootstrap
make check # second pass can't remember value of $ARCH

Instead, we need to change into:

make ARCH=riscv # bootstrap
make ARCH=riscv check

Is this OK? If not, maybe we need some text file to store ARCH.

@jserv
Copy link
Collaborator Author

jserv commented Feb 7, 2021

make config should set and cache the configurations in advance. Therefore, we can specify the target in the first line of config file as following:

/* target: arm */

or

/* target: riscv */

Then, a script in mk directory can read the target from config file. However, once variable ARCH is overridden, the build system should read it. The behavior should be similar to Linux kernel build process.

@jserv jserv closed this as completed in 4de273b Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants