jacobly0 / llvm-project

This fork of the canonical git mirror of the LLVM subversion repository adds (e)Z80 targets. Please refer to the wiki for important build instructions.
https://github.com/jacobly0/llvm-project/wiki
123 stars 15 forks source link

Offset of zero is not printed in the assembler #25

Closed cocoacrumbs closed 2 years ago

cocoacrumbs commented 2 years ago

Hi,

I'm trying to create a complete eZ80 tool chain based on your work and using the Z80 binutils (version 2.37) for assembling and linking (as is done by NuttX as well). For this to work, I applied the changes made by codebje to have bin utils compatible syntax (I had to make some fixes for that as well). My personal hope is that we then get a nice tool chain for an affordable, eZ80 based 8 bit computer which would allow us to port bigger applications (e.g. Lua would be nice but can't be compiled with the Zilog ZDS II tools).

I'm in the process of adapting the ZDS II code to work with this setup to have some minimal C library to build upon.

The first issue I stumbled upon is that when an offset is 0, it's not printed. E.g.:

LEA     HL,IX+0

would be

LEA     HL,IX

Unfortunately, the bin utils assembler (ez80-none-elf-as) doesn't like this syntax and quits.

After a bit of searching I found a simple fix:

diff --git a/llvm/lib/Target/Z80/MCTargetDesc/Z80InstPrinterCommon.cpp b/llvm/lib/Target/Z80/MCTargetDesc/Z80InstPrinterCommon.cpp
index e92f3020b..36bf5228f 100644
--- a/llvm/lib/Target/Z80/MCTargetDesc/Z80InstPrinterCommon.cpp
+++ b/llvm/lib/Target/Z80/MCTargetDesc/Z80InstPrinterCommon.cpp
@@ -104,7 +104,7 @@ void Z80InstPrinterCommon::printAddr(const MCInst *MI, unsigned Op,
   printOperand(MI, Op, OS);
   auto Off = MI->getOperand(Op + 1).getImm();
   assert(isInt<8>(Off) && "Offset out of range!");
-  if (Off > 0)
+  if (Off >= 0)
     OS << " + " << int(Off);
   else if (Off < 0)
     OS << " - " << -int(Off);

Now, the offset is always printed and the assembler is happy.

I do have 2 other, more serious problems:

I'll write up more detailed reports for that.

Koen

jacobly0 commented 2 years ago

Sounds like you were able to fix the issue.

jacobly0 commented 2 years ago

Added some related flags, eventually some "asm format" (or the already existing asm variant) flag should be added that affects the default.

$ cat offset-format.c
void test(void) {
    char *iy;
    asm("": "=y"(iy));
    asm("":: "r"(iy), "r"(iy - 2));
}
$ for zero in "=false" ""; do
      for neg in "false" "true"; do
          ez80-clang -O1 -S offset-format.c -o - -mllvm -z80-print-zero-offset$zero -mllvm -z80-add-negative-offset=$neg
      done
  done | grep "lea\|:$"
_test:
    lea hl, iy - 2
    lea de, iy
_test:
    lea hl, iy + -2
    lea de, iy
_test:
    lea hl, iy - 2
    lea de, iy + 0
_test:
    lea hl, iy + -2
    lea de, iy + 0

I'm just passing the flag value differently to demonstrate both syntaxes, either syntax works with either flag (and also 1/0 instead of true/false).

cocoacrumbs commented 2 years ago

Thanks,

I quickly rebuild the tool chain and tried with the -mllvm -z80-print-zero-offset flag and assembly went fine (it complained when I tried it first without that flag so I'm sure the flag works as intended).