[BUG] - Array of strings

[Jamesbarford]

Describe the bug
The following does not work as intended:

Example Program

U0 Main()
{
  U8 s[][8] = {"hello", "world"};
  "%s\n", s[0]; // will not print hello but do something weird
}

The assembly is very different from what gcc or clang would produce when doing:

#include <stdio.h>

int main(void) {
    char s[][8] = {"hello", "world"};
    printf("%s\n", s[0]); // will print hello
}

Expected behavior
This should print hello

Compiler version
v0.0.8

Commit Hash
ae4fcf9

Desktop (please complete the following information):

• all

[redzimski]

I investigated this a little bit and the fix for this would be to change the leaq instructions that hcc generates into movq:

    # LOAD LEAQ START: AST_TYPE_ARRAY
#   leaq -16(%rbp), %rax <--- old, this would put the pointer to the pointer to "hello" in rax
    movq -16(%rbp), %rax
    # LOAD LEAQ END: AST_TYPE_ARRAY

see the following in gdb:

Breakpoint 1, main () at test.s:28
28		leaq -16(%rbp), %rax
(gdb) s
31		push   %rax
(gdb) i r rax
rax            0x7fffffffe630      140737488348720
(gdb) p *(char **)0x7fffffffe630
$1 = 0x555555555149 <one_dbl+8> "hello"
(gdb)

Also worth noting, gcc embeds the values of the strings into the instructions themselves:

    movabsq	$478560413032, %rax
    movq	%rax, -32(%rbp)

hcc however still puts them into static memory, even though they're stack allocated variables.

What I think is also worth improving here is the memory layout of these programs:

U8 s[][8] = {"hello", "world"};
"%s\n", s[0]; // this doesn't actually work either, but I haven't checked why just yet

#include <stdio.h>
char s[][8] = {"hello", "world"};
int main(void) 
{
    printf("%s\n", s[0]);
}

I think both should generate a single 16-byte block filled with the bytes "hello", 3 zeros, "world", 3 zeros. However, only gcc does that. hcc instead generates this layout:

.L0: .asciz "hello"
.L1: .asciz "world"
# ...
s:
    .quad .L0
    .quad .L1

Which of course is not functionally equivalent.

Also, I've noticed this pattern generated by hcc:

    # Pointer Arithmetic MID
    push   %rax
    movq   $0, %rax
    imul   $8, %rax
    pop    %rcx
    addq   %rax, %rcx
    movq   %rcx, %rax
    # Pointer Arithmetic end

Since the index and size are compile-time, this multiplication can be entirely optimized away. If the index is not a constant, some of this can probably be used (but I'm not that well versed in assembly, especially GAS, so I'm not sure): https://en.wikibooks.org/wiki/X86_Assembly/GNU_assembly_syntax#Address_operand_syntax

[Jamesbarford]

You've done some really good investigative work!

Also worth noting, gcc embeds the values of the strings into the instructions themselves:

I'd suspect this observation is an optimisation as an 8 character string is 8bytes which would fit 64bit integer or a 64bit register. So do have a crack with varying sizes.

I investigated this a little bit and the fix for this would be to change the leaq instructions that hcc generates into movq

If you can find a way to piece that in to x86.c then go for it. The file is pretty hacky.

Since the index and size are compile-time, this multiplication can be entirely optimized away
Yes if we know the values, there is a function called: evalIntArithmeticOrErr(...) in prslib.c that may or may not be useful for that. I've used it with moderate success in asmBinaryOpIntArithmetic(...) in x86.c.

More broadly for the quality of the assembly that is being produced by the compiler, if there are some simple wins to have I'm happy for them to go in.

Though I am aware the assembly produced by the compiler is far from optimal. Creating an IR, when I get some big blocks of time to work on it, are my next big thing to do.

[redzimski]

You've done some really good investigative work!

Thanks!

Also worth noting, gcc embeds the values of the strings into the instructions themselves:

I'd suspect this observation is an optimisation as an 8 character string is 8bytes which would fit 64bit integer or a 64bit register. So do have a crack with varying sizes.

It seems to stop doing it at 129 characters.

More broadly for the quality of the assembly that is being produced by the compiler, if there are some simple wins to have I'm happy for them to go in.

Though I am aware the assembly produced by the compiler is far from optimal. Creating an IR, when I get some big blocks of time to work on it, are my next big thing to do.

I don't know what's your goal and motivation and whatnot for this project but a "get stuff done" approach would be to use something like QBE or LLVM, that way optimizations and platform support are basically free (or at least I'd assume so, I've never actually used them). But that is arguably less fun than doing it yourself :-)

[Jamesbarford]

Yes I'm aware LLVM would solve a lot of problems and broadly make things more robust.

I'm presently not considering using it for this project however. I like that it's possible to clone, build and install this compiler in under 2 seconds. It's also fairly small and quite easy to navigate.

But that is arguably less fun than doing it yourself

This is also a huge factor 😄.

If you're wanting good quality assembly however, a way round would be to use the built in transpiler; hcc -transpile <file>.HC and then use clang. Though the transpiler, I'm sure, has it's own set of bugs...

[redzimski]

I like that it's possible to clone, build and install this compiler in under 2 seconds. It's also fairly small and quite easy to navigate.

Hence why i put QBE first :-)

[redzimski]

the fix for this would be to change the leaq instructions that hcc generates into movq:

This was actually false, things would break when using an array of I32s for example. The thing that made it work is this: https://github.com/krizej/holyc-lang/commit/f050a5a94579b40932a572f971bf30cbc710f564
Whether this is an actual "fix" and not a "it works for this case but it'll break on something else now" I don't know.

As for the memory layout stuff, I think asmDataInternal would need a "parent" parameter so that string nodes can see what the required size is (I don't know how to use the AST stuff yet though). And then also the unnecessary strings in asmDataSection should be skipped somehow. And then change the dereferencing code to support this stuff.