GCC: another multiply bug?

[+chue]

I wasn't sure whether I should post this under the existing GCC thread or start a new thread.

 

I'm putzing around with GCC and I think I may have found another bug when doing multiplies. From what I can tell there have been fixes for other multiply bugs. This may or may not be related.

My environment:

Fedora 25 Linux
GCC 4.4.0 with the tms9900 1.14 patches

I recall having an issue when running Insomniac's installer - there were some directories that the install was expecting to be there but weren't... All I did was create those directories and ran the installer again. Everything seemed to work fine. I don't know if this is relevant to my issue, but just putting it out there.

So on to the code. I have two versions of a function that does a simple calculation using some input parameters.

The first takes all required params and uses them to perform the calculation. This version works just fine:

unsigned int get_vdp_addr_inline(int width, int row, int col)
{
   return row * width + col;
}

Code that calls the above:

unsigned int addr_inline = get_vdp_addr_inline(32, 1, 0);

And now onto the buggy version. In this version the width is stored in a struct. The pointer to the struct is passed into the function instead of the width:

 

typedef struct
{
    int width;
    int height;
    int foo;
    int bar;
} test_rect;

unsigned int get_vdp_addr_struct(test_rect* rect, int row, int col)
{
    return row * rect->width + col;
}

Code that calls the above:

 test_rect rect;
 rect.width = 32;
 rect.height = 24;
 rect.foo = 0;
 rect.bar = 0;
 
 unsigned int addr_struct = get_vdp_addr_struct(&rect, 1, 0);

Like I said this version doesn't give me the correct result. In debugging it with Tursi's classic 99, I see the following assembly generated for the second version of the function above. I added some comments about what it is doing.

                         ; Some memory locations:
                         ; *0000: 83 E0 00 24 83 C0 09 00 ...$....
                         ; *3FF2: 00 20 00 18 00 00 00 00 . ......

                         ; Initial conditions when the function is called:
                         ; R1 (3FF2) is the pointer to "rect"
                         ; R2 is "row" param
                         ; R3 is "col" param

                         ; R1 == 3FF2, R2 == 0001, R3 == 0000

61FA  C042  mov  R2,R1   ; R1 == 0001, R2 == 0001, R3 == 0000
​                         ; The above move instruction just blasted the "rect" pointer... this will not work.                
61FC  3851  mpy  *R1,R1  ; R1 == 0000, R2 == 83E0, R3 == 0000
​                         ; This multiply does not make sense.
​                         ; Seems to be using the wrong registers.                                 
61FE  C042  mov  R2,R1   ; R1 == 83E0, R2 == 83E0, R3 == 0000                
6200  A043  a    R3,R1   ; R1 == 83E0, R2 == 83E0, R3 == 0000                
6202  045B  b    *R11    ; return to caller, INCORRECT result in R1                                   

In case it's relevant, these are the GCC / Linker flags I am using:

C_FLAGS=\
  -O2 -std=c99 -s --save-temp -fno-builtin -Wall -Wstack-protector
LDFLAGS=\
 --section-start .text=6000 --section-start .data=2000

To me, it looks like a bug in the compiler output; however, it is possible it's some kind of user error. Hopefully Insomniac will see this at some point.

 

I think my workaround for now is to hand code some assembly in place of the buggy function.

 

Edited May 3, 2017 by chue

[+chue]

By the way, awesome work on the compiler Insomniac!

[+chue]

I figured out the workaround - I combined the two versions of the function above so that the pointer dereference and the multiply happen separately:

__attribute__ ((noinline)) unsigned int get_vdp_addr_inline(int width, int row, int col)
{
   return row * width + col;
}

unsigned int get_vdp_addr_struct(test_rect* rect, int row, int col)
{
    return get_vdp_addr_inline(rect->width, row, col);
}

Calling code:

 test_rect rect;
 rect.width = 32;
 rect.height = 24;
 rect.foo = 0;
 rect.bar = 0;
 
 unsigned int addr_struct = get_vdp_addr_struct(&rect, 1, 0);

[+chue]

I seemed to have hit another small unrelated bug. In this one it doesn't look like globals are being properly initialized. I have a simple test case here that uses the header.asm and crt0.asm files from the "hello.tar.gz" archive at the start of the GCC thread: http://atariage.com/forums/topic/164295-gcc-for-the-ti/?p=2028632

 

In the following code, I always see "FAIL" written to the screen. This means the global variable "heap_start" is not getting set properly. From what I can tell, it is initialized to zero instead of 0xF000.

 

#define VDP_READ_DATA_REG (*(volatile char*)0x8800)
#define VDP_WRITE_DATA_REG (*(volatile char*)0x8C00)
#define VDP_ADDRESS_REG (*(volatile char*)0x8C02)
#define VDP_READ_FLAG 0x00
#define VDP_WRITE_FLAG 0x40
#define VDP_REG_FLAG 0x80

static void vdp_copy_from_sys(int index, char* src, int size)
{
    volatile char* end = src + size;
    VDP_ADDRESS_REG = index | VDP_WRITE_FLAG;
    VDP_ADDRESS_REG = (char)(index >> ;
    while(src != end)
        VDP_WRITE_DATA_REG = *src++;
}

// ----------------------------------------

#define HEAP_START 0xF000
void* heap_start = (void*)HEAP_START;

// something to prevent the compiler from
// turning the global variable into a constant.
void inc_heap_start()
{
    heap_start += 8;
}

int main()
{
    if (heap_start == (void*)HEAP_START)
        vdp_copy_from_sys(32, "OK", 2);
    else
        vdp_copy_from_sys(32, "FAIL", 4);
    inc_heap_start();
   
    while (1) {}
}

[+jedimatt42]

Not sure that is a bug, or just a TI thing. The crt0.asm code starts C execution in your main. The line initializing your global variable isn't in main. So it isn't called.

 

If you initialize in main, then that'll work around it. But, you might look at the generated map file or .s and see if the global initializers are placed in another function. Then mod crt0 to call that function.

 

I'm sure it is more complicated than that...

 

-M@

[+chue]

Matt, you're correct that there are many ways to fix it.

 

I suspect the "right" way to go is to put something in crt0. I've seen some examples of this being done, one example here (http://atariage.com/forums/topic/241386-creating-bank-switched-cartridges-with-gcc/?p=3621300).

 

I'll post my solution if/when I get it working. It's a little on the back burner right now as I'm also trying to understand bank switching... but that's perhaps for another thread.

 

chue

Edited May 15, 2017 by chue

[insomnia]

I don't know if this is helpful, but I've run into a problem like this before where the root cause was a non-word-aligned .data section.

 

The memory initialization code in crt0 copies words, so a misalignment like this results in a mess of strangely-behaving code. I've tried to address this in my newer projects by making sure the linker does the alignment for me.

 

This is just a guess, I haven't had a chance to look at any code lately. I really should either update the hello example or post an improved crt0 sometime soon.

[+jedimatt42]

The crt0.asm in hello.tar.gz doesn't have (any) the dseg copy routine in it, like I've seen in the crt0_ea5.asm from libti99.

 

https://github.com/tursilion/libti99/blob/master/crt0_ea5.asm

 

I haven't tried to get that code to work in a cartridge build context yet... hmm.

 

-M@

[+jedimatt42]

When I try to link a cart with the magic _init_data 'structure' at the end of crt0... the elf2cart doesn't fill in those values in the ROM. all zeros in the hex editor.

 

I'm guessing that ELF2CART doesn't fill in this magic _init_data table?

 

-M@

[+chue]

The crt0.asm in hello.tar.gz doesn't have (any) the dseg copy routine in it, like I've seen in the crt0_ea5.asm from libti99.

 

-M@

 

Ah of course, I should have thought to also look in libti99! I'll have to check that out.

 

 

When I try to link a cart with the magic _init_data 'structure' at the end of crt0... the elf2cart doesn't fill in those values in the ROM. all zeros in the hex editor.

 

I'm guessing that ELF2CART doesn't fill in this magic _init_data table?

 

 

If you look at the libti makefile, you'll see the use of a different elf utility called elf2ea5 and not elf2cart. Perhaps that is the difference here? I do have the source to elf2ea5 but need to investigate further.

[+chue]

I don't know if this is helpful, but I've run into a problem like this before where the root cause was a non-word-aligned .data section.

 

The memory initialization code in crt0 copies words...

 

This is just a guess, I haven't had a chance to look at any code lately. I really should either update the hello example or post an improved crt0 sometime soon.

 

My data section does appear to be aligned, but as Matt says above it looks like there isn't any data copy code in crt0.

 

It would be nice to have data init working "out of the box" at some point.

[+chue]

 

I seemed to have hit another small unrelated bug. In this one it doesn't look like globals are being properly initialized....

 

 

OK I've been playing around with this a little more. This appears to be a bug related to the "elf2cart" utility. It's entirely possible I'm not using the right version. In any case, I looked at both binaries (.elf, and .cart) generated during the build process and the elf contains the initialization data while the cart does not.

 

So even if crt0 contains code to initialize variables, it won't work because the data isn't there!

 

I'm dropping this for now as I'm more interested in binaries that follow The Mole's bank switching build process (http://atariage.com/forums/topic/241386-creating-bank-switched-cartridges-with-gcc/?p=3294624) - this does not output elf files so would not be affected by the init bug.

Edited May 29, 2017 by chue

[+chue]

I have confirmed that Insomnia's 1.15 patches (from here: http://atariage.com/forums/topic/164295-gcc-for-the-ti/?p=2028632) resolve the multiply bug above. Thanks again Insomnia!

Edited June 3, 2017 by chue