GCC for the TI

+khanivore · January 29

On 1/27/2024 at 3:22 AM, Tursi said:

The alternative is probably to stop using AI for byte math (instead using AB/SB), that way the condition codes will always be correct.

Agreed. I’ve been experimenting with constant pools and I think I have them working. They should simplify cases like this and save a few bytes too. So instead of:

    LI r1,>5500
    CB @xyz, r1

we can have:

    CB @xyz,@LC2
    ...
LC2 byte >55

The same should work for AB, SB, etc

+khanivore · January 29

On 1/28/2024 at 3:41 PM, Tursi said:

Why did it promote the comparison?

I’ve seen this before when debugging the andhi3 issue. It upgrades values to 16 bit even though all parameters are bytes,

Interestingly, gcc13 didn’t do that. I’ll test that tomorrow.

+FarmerPotato · January 31

On 1/29/2024 at 3:05 PM, khanivore said:

I’ve seen this before when debugging the andhi3 issue. It upgrades values to 16 bit even though all parameters are bytes,

Interestingly, gcc13 didn’t do that. I’ll test that tomorrow.

I was under the impression that the ANSI C standard first said to promote char to int for operators. But I couldn't find justification for it in K&R 2.0 (I prefer books, it was handy.)

After much looking: this StackOverflow answer seems adequately sourced from C99 standard. But I wasn't sure.

Here's what I found on cppreference.com.

Quote

Integral promotion

prvalues of small integral types (such as char) and unscoped enumeration types may be converted to prvalues of larger integral types (such as int). In particular, arithmetic operators do not accept types smaller than int as arguments, and integral promotions are automatically applied after lvalue-to-rvalue conversion, if applicable. This conversion always preserves the value

There, arithmetic operators don't work on chars, they work on ints. But logical operators? Is andhi3 invoked by a logical operator like ~

("integral type" is the type domain including char, int, long etc)

+khanivore · February 1

I've pushed various changes to the 1.29 branch https://github.com/mburkley/tms9900-gcc/tree/1.29

Unit tests are passing so far and code is a bit smaller (by linecount at least).

(patch files haven't been updated yet, just md and associated files)

JasonACT · February 2

11 hours ago, khanivore said:

changes to the 1.29 branch

This doesn't look right to me:

void tttt(void * t) {
    int (*t2) (unsigned char c, unsigned int len) = t;
    t2 (32,32);
}

Produces:

	def	tttt
tttt
	mov  r1, r3
	li   r2, >20
	movb r2, r1 ; tms movqi
	swpb r1 ; movqi byte correct
	b    *r3 ; tail call
	.size	tttt, .-tttt

Edited February 2 by JasonACT
-Os

Fabrizio Caruso · February 2

Hi everyone!
How can I test the latest changes?
I run git pull to pull the latest changes -> OK.

Then if I rerun ./install.sh <path> it seems to be doing nothing.

If before that I run cleanpatches.sh, then when I run ./install.sh
I get the prompt "Reversed (or previously applied) patch detected! Assume -R? [n] y", to which I can answer y or no and in both cases I get failures.

+khanivore · February 2

3 hours ago, Fabrizio Caruso said:

Hi everyone!
How can I test the latest changes?
I run git pull to pull the latest changes -> OK.

Then if I rerun ./install.sh <path> it seems to be doing nothing.

If before that I run cleanpatches.sh, then when I run ./install.sh
I get the prompt "Reversed (or previously applied) patch detected! Assume -R? [n] y", to which I can answer y or no and in both cases I get failures.

Hi @Fabrizio Caruso it is best to do a clean build. Either remove or rename your existing build directory before you run the install script. The version on main is 1.28. The 1.29 version is still in review and test and hasn't been merged to main yet and doesn't have a patch file yet.

+khanivore · February 2

6 hours ago, JasonACT said:

This doesn't look right to me:

void tttt(void * t) {
    int (*t2) (unsigned char c, unsigned int len) = t;
    t2 (32,32);
}

Produces:

	def	tttt
tttt
	mov  r1, r3
	li   r2, >20
	movb r2, r1 ; tms movqi
	swpb r1 ; movqi byte correct
	b    *r3 ; tail call
	.size	tttt, .-tttt

If doing a post move byte swap then it should emit a MOV instead of a MOVB. I've changed this now so output is now this:

tttt
        mov  r1, r3
        li   r2, >20
        mov  r2, r1 ; tms movqi
        swpb r1 ; movqi byte correct
        b    *r3 ; tail call
        .size   tttt, .-tttt

+khanivore · February 2

On 1/31/2024 at 5:46 PM, FarmerPotato said:

Is andhi3 invoked by a logical operator like ~

No, it has a separate insn called one_cmplhi2 which emits the INV opcode. There is a one_cmplqi2 as well for bytes, but there is no separate byte INV instruction in tms of course so it emits the same instruction.

The issue is that when extending an unsigned char, it emits SRL but if extending a signed char it would have emitted SRA which extends the sign bit and it would have worked ok.

Tursi · February 2

I pulled the latest branch, deleted my build folder, and rebuilt -- because of the patching I was not 100% sure that was the right answer?

Unfortunately, though most tests passed, Super Space Acer showed regressions again. I got the "top of screen" movement bug and the "broken noise playback" bug again.

All with Os and no-function-cse

Top of screen was the bug we looked at back on January 1 which was writing bytes to memory as words and then using SWPB to "fix" them (which of course terribly breaks writes to my sprite table).

    movb @SpriteTab+1, r1   <---- get value from table as byte
    mov  @playerXspeed, r2
    movb r1, r4
    srl  r4, 8
    mov  r2, r3
    ai   r3, >FF20
    neg  r3
    c    r4, r3
    jhe  L101
    swpb r2
    ab   r1, r2
    mov  r2, @SpriteTab+1   <---- write it back as a word (CORRUPTS MEMORY)
    swpb @SpriteTab+1       <---- swap the bytes in memory (EVEN WORSE!  )
    jmp  L101

We tried various things in movqi but I feel like only the updated constraints actually fixed the issue.

Broken noise was using AI for byte math, and that's confirmed in the assembly:

    movb *r1, r9    <--- get countdown value as a byte
    inc  r1
    mov  r1, @pSfx
    movb r9, r0
    jne  L127
    clr  r1
    mov  r1, @pSfx
    jmp  L124
L127
    mov  @pSfx, r1
    bl   @processSID
    mov  r1, @pSfx
    ai   r9, >FF00  <--- but decrement it as a word
    jne  L127

Need to use AB instead (in my local hack I just loaded the value into R0 and used that )

Edited February 2 by Tursi

+khanivore · February 3

6 hours ago, Tursi said:

We tried various things in movqi but I feel like only the updated constraints actually fixed the issue.

Hmmm … I think you’re right … let me try and revert tomorrow

Fabrizio Caruso · February 3

Sorry guys if this question is a bit out of scope:
Is the main branch still compatible with libti99 (https://github.com/tursilion/libti99)?

On a different but still related topic, do we have any chance that at least some basic input/output routines if not all libti99 could be distributed with the compiler to make things simpler for the user?

Tursi · February 3

5 hours ago, Fabrizio Caruso said:

Sorry guys if this question is a bit out of scope:
Is the main branch still compatible with libti99 (https://github.com/tursilion/libti99)?

On a different but still related topic, do we have any chance that at least some basic input/output routines if not all libti99 could be distributed with the compiler to make things simpler for the user?

It may or may not be. If we can get these last bugs fixed I'll be happy to build and certify libti99 -- I've been mostly focusing on the newer libti99all, but I haven't pushed that out yet.

+khanivore · February 3

21 hours ago, Tursi said:

I pulled the latest branch, deleted my build folder, and rebuilt -- because of the patching I was not 100% sure that was the right answer?

Sorry @Tursi I missed this bit. I hadn't updated the patch file so if you just ran install you wouldn't have picked up the latest changes. I've updated and pushed the patch file now to the 1.29 branch.

+khanivore · February 7

I haven't received any more feedback on 1.29 or seen any more failures, so I've gone ahead and merged it to the main branch. The updates in 1.29 are :

gcc patch 1.29
* Add function to swpb before or after MOV[B] if subreg offset seen
* Fix operand count for one_cmpl ops
* Changed compare insns "cmphi" and "cmpqi" to be insn_and_split
* Added peepholes to remove SRL Rx,8 ; SWPB
* Added R12 thru R15 to save regs to improve general reg availability
* Added literal constants for movqi and movhi to avoid LI to a scratch reg
* Removed sub_const_hi and associated peepholes
* Removed NEG from subtract operations that had reversed operands
* Added debugs to peepholes to find which ones are actually used

Tursi · February 7

Sorry, it takes a couple of hours to rebuild the compiler and all my tools and then test all the functions, and I haven't had a couple of hours this week.

+khanivore · February 7

3 hours ago, Tursi said:

Sorry, it takes a couple of hours to rebuild the compiler and all my tools and then test all the functions, and I haven't had a couple of hours this week.

No worries. I'm heading off myself for a few days and just wanted to get the changes merged before I go. But please do let me find anything in your testing. Thanks.

+chue · February 7

9 hours ago, khanivore said:

I haven't received any more feedback on 1.29

My unit tests all pass, so it looks good to me.

I thought I had found a problem, but then I realized I didn't recompile one of my libs. After that, everything was fine.

Thank you!

Tursi · February 8

Deleted the build folder, pulled main, and rebuilding using the install script.

Compiler crashed building libti99all. All my stuff depends on that so I couldn't get any further.

Quote

~/newtms9900-gcc/newgcc9900/bin/tms9900-gcc -c ../vdp_settext.c -O2 -std=c99 -DTI99 -s --save-temp -I../ -fno-builtin -fno-function-cse -o vdp_settext.o
../vdp_settext.c: In function ‘set_text_raw’:
../vdp_settext.c:23: internal compiler error: in do_SUBST, at combine.c:676
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[1]: *** [../Makefile.ti99:76: vdp_settext.o] Error 1
make[1]: Leaving directory '/mnt/d/work/libti99ALL/buildti'
make: *** [Makefile:49: ti] Error 2

vdp_settext.zip

(Numerous other files did build before it got here.)

Edited February 8 by Tursi

+khanivore · February 8

2 hours ago, Tursi said:

Compiler crashed building libti99all. All my stuff depends on that so I couldn't get any further.

Very strange. I'm not seeing that here. Even with a clean build. I'll have a look at combine.c .....

(edit: nvm, wasn't using code in the attached zip, seeing it now)

Edited February 8 by khanivore

JasonACT · February 8

28 minutes ago, khanivore said:

Very strange. I'm not seeing that here. Even with a clean build. I'll have a look at combine.c .....

I'm getting it here, except mine says combine.c line 677 (but I'm using a slightly older GCC to build this really old GCC).

JasonACT · February 8

It appears to boil down to the return value:

unsigned char set_text_raw() {
    return 0x80;
}

Other values with the high-bit not set generate code OK. Like 0x40 through to 0x01 and 0x00.

Edit: If I remove the assert, it generates OK and the byte value is 128.

Edited February 8 by JasonACT

+khanivore · February 8

It looks like internally QI is always treated as signed ? If I remove the masks before generating the constants then it generates -16 instead of 240 which works fine.

Quote

--- a/dev/gcc-4.4.0/gcc/config/tms9900/tms9900.md
+++ b/dev/gcc-4.4.0/gcc/config/tms9900/tms9900.md
@@ -416,7 +416,7 @@
}

tms9900_inline_debug ("; movqi replace const with label\n");
- operands[1] = force_const_mem (QImode, GEN_INT(INTVAL (operands[1]) & 0xff));
+ operands[1] = force_const_mem (QImode, operands[1]);
}

emit_insn (gen_tms9900_movqi (operands[0], operands[1]));
@@ -491,7 +491,7 @@
if (!REG_P (operands[0]))
{
tms9900_inline_debug ("; movhi replace const with label\n");
- operands[1] = force_const_mem (HImode, GEN_INT(val));
+ operands[1] = force_const_mem (HImode, operands[1]);
}
}

@@ -1953,7 +1953,7 @@
if (CONST_INT_P (operands[2]))
{
tms9900_inline_debug ("; addqi3 replace const with label\n");
- operands[2] = force_const_mem (QImode, GEN_INT (INTVAL (operands[2]) & 0xff));
+ operands[2] = force_const_mem (QImode, operands[2]);
}

emit_insn (gen_tms9900_addqi3 (operands[0], operands[1], operands[2]));
@@ -2045,7 +2045,7 @@
if (CONST_INT_P (operands[2]))
{
tms9900_inline_debug ("; subqi3 replace const with label\n");
- operands[2] = force_const_mem (QImode, GEN_INT (INTVAL (operands[2]) & 0xff));
+ operands[2] = force_const_mem (QImode, operands[2]);
}

emit_insn (gen_tms9900_subqi3 (operands[0], operands[1], operands[2]));

+khanivore · February 8

The assert triggers because the function trunc_int_for_mode() seems to always return sign extended values. I noticed this when I added some debugs:

oldmode=QI old=15 new=fffffff0, trunc=fffffff0
oldmode=QI old=1 new=fffffff0, trunc=fffffff0
oldmode=QI old=1 new=f0, trunc=fffffff0
(assert here)

JasonACT · February 8

1 hour ago, khanivore said:

If I remove the masks before generating the constants then it generates -16 instead of 240 which works fine.

Seems fine to me, giving appropriate warnings when it's out of bounds, but still generating the code I expect.

GCC for the TI

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Recently Browsing 0 members