AVR Freaks

Theusch, it only does the crazy long thing when one of the arguments is unsigned.

Christop, thanks - I will download your example from pastebin and investigate my makefile to see if I am breaking it there somehow.

Straight into another question.

I have a table in flash that is aligned to a 256 byte boundary for speed of access in my ASM

LDFLAGS += -Wl,--section-start=.trigtableflash=0x00002100

const uint8_t SinCosTable[] __attribute__ ((section (".trigtableflash"))) =  {

and the ASM for sin() and cos() looks like this

; int8_t CosFastC(uint8_t angle)
;
; Returns the cosine of the angle
;
; Inputs
;		Angle (0..255) in r24
; Returns
;		cos(angle) as signed 8 bit value in r24
; Trashes
;		R24
;		R26:27

CosFastC:

     subi    r24, (-(64))			; COS is 90 degrees out of phase with SIN

SinFastC:

     mov     r30, r24				; Get the offset in the SIN table
     ldi     r31, hi8(SinCosTable)
     lpm     r24, Z				; Read value from table into r24
     ret

I don't think this one is likely - but is there are a way to make the C code not do

R = pgm_read_byte(&SinCosTable[objData->r]);
     adiw    r26, 0x05	; 5
     ld      r30, X
     sbiw    r26, 0x05	; 5
     ldi     r31, 0x00	; 0
     subi    r30, 0x00	; 0
     sbci    r31, 0xDF	; 223
     lpm     r30, Z

It's the LDI/SUBI/SBCI that the alignment was intended to get rid of.

The compiler KNOWs the table is 8 bit boundary, can you convince it that it does not need to subtract ZERO from the address?

It's a fairly big game project for the Uzebox, I am trying to get as much of the non video rendering stuff into C as possible for other people to be able to read and learn from it.

The section of the game that renders all the video to screen and draws all the lines is in ASM as C won't be able to do that.

However the sections of the game that do

Object management

Object movement

Collision detection

Game logic

Are not so time critical and are mostly in C already.

The Sin/Cos thing is called often enough that I will code it as inline ASM though.  The inline ASM thing is not as intimidating as it first looked now I have bothered to work it out.

Yes - anything that looks uglier in C to make it optimal won't be used.

For example the above code where #defining an inline ASM macro only saved one clock over

xAdd = (xAdd * Z) >> 8;
yAdd = (yAdd * Z) >> 8;

is out.  But the inline ASM to replace

xAdd = (SinFastC(thisLaneAngle + edgeAngle) * laneWidth) >> 8;
yAdd = (CosFastC(thisLaneAngle + edgeAngle) * laneWidth) >> 8;

is in.  Because not only is it a lot of clocks, it also is called quite often.

Also any ASM code is pretty well explained when people do want to look at it - as an example

; fast_line_convert_x0_y0_into_VRAM_address
;
; converts the X0 and Y0 address passed in r24 and r22 into a VRAM memory
; location and leaves this result in R26:27 (VRAM_Address)
;
; Inputs
;            Y0 address = R22
;            X0 address = R24
; Outputs
;            VRAM_Address = R26:27 (X)
;
; Requires that the constants 4 and 32 are pre-loaded in R10, R11
;
; Trashes R0:1

.macro fast_line_convert_x0_y0_into_VRAM_address
     mul     r22, r10                               ; Multiply Y0 by 4   y7y6y5y4y3y2y1y0      => .0.0.0.0.0.0y7y6:y5y4y3y2y1y0.0.0
     movw    r26, r0                                ; move the 16 bit result into VRAM_Address
     andi    r26, 0b11100000                        ; clear out the bits that are used for Xn     .0.0.0.0.0.0y7y6:y5y4y3.0.0.0.0.0
     mul     r24, r11                               ; Multiply X0 by 32  x7x6x5x4x3x2x1x0      => .0.0.0x7x6x5x4x3:x2x1x0.0.0.0.0.0
     or      r26, r1                                ; OR X7..3 into low byte of VRAM_Address      .0.0.0.0.0.0y7y6:y5y4y3x7x6x5x4x3
     subi    r27, hi8(-(vram))                      ; Add the base address of VRAM                .0.0.0.0.1.1y7y6:y5y4y3x7x6x5x4x3
.endm

And any code that is almost a direct equivalent of something from C has the C code above it in comments.

/*
void renderObjects(void){
     uint8_t i;
     ObjectDescStruct *Current;

     drawFunctionPointer_t drawFunction;

     for(i = 0; i < MAX_OBJS; i++) {
          Current = (ObjectDescStruct*)&ObjectStore[i];
          if(Current->obType != OBJ_EMPTY) {
               drawFunction = (drawFunctionPointer_t)pgm_read_word(&drawFunctionPointers[Current->obType]);
               drawFunction(Current);
          }
     }
}
*/

renderObjects:
     fast_line_entry_C                               ; save all the registers C does not want trashed
     fast_line_entry                                 ; set up all the registers for the draw line routines

     ldi		r28, lo8(ObjectStore)        ; Get the base address of the ObjectStore[] array
     ldi		r29, hi8(ObjectStore)
renderObjectsLoop:
     ld      r30, Y                                  ; Get the Object Type of the current object (r30 = ObjectStore[i].ObjType)
     cpi     r30, 0x00                               ; If the object type is <zero>
     breq    renderObjectsSkip                       ; we don't need to draw anything
     add     r30, r30                                ; Multiply the object number by 2 (flash addressing)
     clr     r31                                     ; and clear ZH (NB: Object Type can not be > 128 for this to work)
     subi    r30, lo8(-(drawFunctionPointers))       ; Add the base address to the function pointer table in PROGMEM
     sbci    r31, hi8(-(drawFunctionPointers))
     lpm     r26, Z+                                 ; Get the address of the code/routine that draws the given
     lpm     r27, Z+                                 ; object type into Register X
     movw    r30, r26                                ; move that value into Z ready for the ICALL
     icall                                           ; Call the routine that draws the object

renderObjectsSkip:
     adiw    r28, 0x08                               ; add 8 to Y to point to the next element of ObjectStore[]
     cpi     r29, 0x04                               ; if the address of Y hits 0x0400 we are at the end of the object store
     brne    renderObjectsLoop                       ; else loop back

     fast_line_exit_C                                ; restore the registers C needs to not be trashed
     ret

It is a fairly famous game I am porting to the Uzebox and I suspect a few people to be interested in how I managed it.
 

I think this will be the last one.  I have picked most of the low hanging fruit.  In fact even with this one I am probably going to leave it in C.  I am curious if there is a way to get C to use MUL or ignore the MSB.

I have a 16 bit pointer that is the address of a structure in an array.  I need to go back to the index of the array

In effect I am doing

i = ( (uint16_t)(objectPointer) - (uint16_t)(&objectArray[0]) ) / sizeOf(object);

Now I know

• the result of this answer can be at most 59 (a 6 bit number)
• The sizeOf() the structure is 8 bytes
• The base address of the array is 0x0220 (fixed address from a section)
• Therefore the highest address the pointer could be is 0x03F8 (0x220 + 59*8)
• Therefore the highest value the result of the /8 can be is 127 (a 7 bit number)

So I know if I do the divide by 8 first, I can just use the lowest 8 bits and minus 0x44 (0x0220 / 8 = 0x44) hence the most optimal C I can make is

i = ((uint8_t)((uint16_t)(ob2) / 8)) - 0x44;

which gets compiled to

ldi   r24, 0x03	; 3          Do the lsr/ror loop 3 times
lsr   r23
ror   r22
dec   r24
brne  .-8
subi  r22, 0x44	; 68

Which is 16 clocks.

In ASM I can

ldi   r21, 32        ; value to multiply by
mul   r23, r21
mov   r22, r1        ; get the lowest 5 bits of result into destination
mul   r24, r21
or    r22, r0        ; or in the highest 2 useful bits into destination
eor   r1,  r1
subi  r22, 0x44 ; 68 

that keeps the full 8 bits of the result in 9 clocks OR because I know the answer is only 7 bits I can

lsr   r23
ror   r22
lsr   r23
ror   r22
lsr   r22    ; last itteration does not need ror as result is 7 bits
subi  r22, 0x44

which is the answer I need in this specific case in 6 clocks.

Any way of getting C to understand I only have 7 bits and the short LSR/ROR trick can be taken?  Or even make it do the full 3 LSR/ROR (no BRNE.-8) and blow the code size out by 1 word and still get the answer in 7 clocks?
 

you mean to

andi   r22, 0x03
ori    r22, 0x04
lsr    r23
ror    r22
brcc   .-6
subi   r22, 0x44

OK - another one.

extern uint8_t spibyte_ff(void);

int mmcGetInt(void){
    return( spibyte_ff() | spibyte_ff()<<8 );
}

comes out as

int mmcGetInt(void){
    push    r28

return( spibyte_ff() | spibyte_ff()<<8 );

    call    0xa9c4      ; 0xa9c4 <spibyte_ff>
    mov     r28, r24
    call    0xa9c4      ; 0xa9c4 <spibyte_ff>
    mov     r18, r28
    ldi     r19, 0x00   ; 0

}
    movw    r20, r18
    or      r21, r24
    movw    r24, r20
    pop     r28
    ret

and I would like it to look more like

    call    spibyte_ff
    push    r24
    call    spibyte_ff
    pop     r25
    ret

Technically in the ASM version I don't need to push/pop as I know R25 is untouched by spibyte_ff, but I am keeping it C/ABI compatible and assuming R25 could get trashed.

I'm using GCC 4.9.2 with -Os.  I have an almost full Mega644 and am going through C code and trying to save an extra few K to fit in another song now.

Had already tried

uint16_t i;

((char*)&i)[0] = mmc_get_byte();
((char*)&i)[1] = mmc_get_byte();
return i;

and

return( mmc_get_byte() + mmc_get_byte()<<8 );

and

return( mmc_get_byte() + mmc_get_byte()*256 );

The one with OR ended up the best.

Just wanted to check here that there was not something else I didn't know about to help.  I have learned a LOT about how the C compiler thinks here, but I still know very little.

OK - this one has just moved to a (dot)s file.  Using the knowledge that mmc_get_byte only touches r24, r30, r31 AND the fact that GetInt is a subset of GetLong let me shrink it a bit.

.section .text.mmcGetLong
mmcGetLong:
    rcall   mmc_get_byte     ; First byte from SD card straight to R22
    mov     r22, r24
    rcall   mmc_get_byte     ; Second byte from SD card straight to R23
    mov     r23, r24
                             ; Fall through to GetInt: to receive 3rd and 4th bytes
.section .text.mmcGetInt
mmcGetInt:
    rcall   mmc_get_byte     ; First byte from SD card to temp location in R20 (3rd byte of GetLong)
    mov     r20, r24
    rcall   mmc_get_byte     ; Second byte from SD card to temp location in R21 (4th byte of GetLong)
    mov     r21, r24    
    movw    r24, r20         ; Move R20:21 to the R24:25 location C is expecting it
    ret

.section .text.mmcGetChar    ; Simple fall through to get_byte
mmcGetChar:

.section .text.mmc_get_byte
mmc_get_byte:
    rcall   spibyte_ff
    .
    .
    .

And no one picked me up on the obvious saving of another 4 bytes I missed. (I noticed it myself when I was cleaning up comments)

.section .text.mmcGetLong
mmcGetLong:
  rcall  mmcGetInt       ; First two bytes from SD card and move to R22:23
  movw   r22, r24
                         ; Fall through to GetInt to receive 3rd and 4th bytes
.section .text.mmcGetInt
mmcGetInt:
  rcall  mmc_get_byte    ; First byte from SD card to temp location in R20 (3rd byte of GetLong)
  mov    r20, r24
  rcall  mmc_get_byte    ; Second byte from SD card to temp location in R21 (4th byte of GetLong)
  mov    r21, r24
  movw   r24, r20        ; Move R20:21 to the R24:25 location C is expecting it
  ret

Thanks joey,

I am pretty sure that 4 bit will sound fine for the sample as it is played at the same time as 3 other channels of 8 bit generated sound in the music.

The problem for Uze will be if he can fit the 3 note channels, the one noise channel and the 4 bit WAV channel into the 130 clocks that HSync runs for.

<Thread Necromancy>

Hi again all,

Need help giving the compiler hints again that are human friendly.

The following code compiles and works fine.  Just a bit slow.  As I have said previously I like to keep code in C rather than moving it to ASM if I can keep performance within 5% or so.  Unless of course the C is more confusing and less readable than the ASM version.

typedef struct {
	uint8_t next;
	uint8_t col;
	uint8_t y1;
	uint8_t x1Lo;
	uint8_t x1;
	int16_t m;
	uint8_t y2;
} GradLineStruct;

void Add_Line(uint8_t col, uint8_t y1, uint8_t x1, int16_t m, uint8_t y2){

	if (linkNext == 255) { return; }                                        // If we have run out of space to add new lines then fail

	if (linkNext != 0) { gradLines[linkNext].next = linkNext + 1;	};      // If we are pointing to array index[0] then don't update "next"
                                                                            //   index[1] is the first valid entry that can be used.
																			
	linkNext++;                                                             // Update the "Next" to point to the current index (it was pointing to last)
	
	gradLines[linkNext].next = 0;                                           // Insert the values
	gradLines[linkNext].col  = col;
	gradLines[linkNext].y1   = y1;
	gradLines[linkNext].x1   = x1;
	gradLines[linkNext].x1Lo = 0;
	gradLines[linkNext].m    = m;
	gradLines[linkNext].y2   = y2;
}

And the LSS is

void Add_Line(uint8_t col, uint8_t y1, uint8_t x1, int16_t m, uint8_t y2){
    2a18:	0f 93       	push	r16

	if (linkNext == 255) { return; }                                        // If we have run out of space to add new lines then fail
    2a1a:	a0 91 c7 01 	lds	r26, 0x01C7	; 0x8001c7 <linkNext>
    2a1e:	af 3f       	cpi	r26, 0xFF	; 255
    2a20:	d9 f0       	breq	.+54     	; 0x2a58 <Add_Line+0x40>
    2a22:	e1 e0       	ldi	r30, 0x01	; 1
    2a24:	ea 0f       	add	r30, r26

	if (linkNext != 0) { gradLines[linkNext].next = linkNext + 1;	};      // If we are pointing to array index[0] then don't update "next"
    2a26:	aa 23       	and	r26, r26
    2a28:	39 f0       	breq	.+14     	; 0x2a38 <Add_Line+0x20>
    2a2a:	98 e0       	ldi	r25, 0x08	; 8
    2a2c:	a9 9f       	mul	r26, r25
    2a2e:	d0 01       	movw	r26, r0
    2a30:	11 24       	eor	r1, r1
    2a32:	a0 50       	subi	r26, 0x00	; 0
    2a34:	bc 4f       	sbci	r27, 0xFC	; 252
    2a36:	ec 93       	st	X, r30
                                                                            //   index[1] is the first valid entry that can be used.
																			
	linkNext++;                                                             // Update the "Next" to point to the current index (it was pointing to last)
    2a38:	e0 93 c7 01 	sts	0x01C7, r30	; 0x8001c7 <linkNext>
	
	gradLines[linkNext].next = 0;                                           // Insert the values
    2a3c:	98 e0       	ldi	r25, 0x08	; 8
    2a3e:	e9 9f       	mul	r30, r25
    2a40:	f0 01       	movw	r30, r0
    2a42:	11 24       	eor	r1, r1
    2a44:	e0 50       	subi	r30, 0x00	; 0
    2a46:	fc 4f       	sbci	r31, 0xFC	; 252
    2a48:	10 82       	st	Z, r1
	gradLines[linkNext].col  = col;
    2a4a:	81 83       	std	Z+1, r24	; 0x01
	gradLines[linkNext].y1   = y1;
    2a4c:	62 83       	std	Z+2, r22	; 0x02
	gradLines[linkNext].x1   = x1;
    2a4e:	44 83       	std	Z+4, r20	; 0x04
	gradLines[linkNext].x1Lo = 0;
    2a50:	13 82       	std	Z+3, r1	; 0x03
	gradLines[linkNext].m    = m;
    2a52:	36 83       	std	Z+6, r19	; 0x06
    2a54:	25 83       	std	Z+5, r18	; 0x05
	gradLines[linkNext].y2   = y2;
    2a56:	07 83       	std	Z+7, r16	; 0x07
}
    2a58:	0f 91       	pop	r16
    2a5a:	08 95       	ret

I have no idea WHY the PUSH R16 at the start of the routine and the POP R16 at the end OR the superfluous SUBI R31, 0x00 and OER R1,R1 but that's not the question here.

If you look at the C code - you can see the two array access locations are sequential.  I have been able to trick the compiler into using the larger indexes to access the second one like this

typedef struct {
	uint8_t next;
	uint8_t col;
	uint8_t y1;
	uint8_t x1Lo;
	uint8_t x1;
	int16_t m;
	uint8_t y2;
} GradLineStruct;

typedef struct {
	uint8_t last;
	uint8_t dummy1;
	uint8_t dummy2;
	uint8_t dummy3;
	uint8_t dummy4;
	int16_t dummy5;
	uint8_t dummy6;
	uint8_t next;
	uint8_t col;
	uint8_t y1;
	uint8_t x1Lo;
	uint8_t x1;
	int16_t m;
	uint8_t y2;
} GradLineStructX2;

void Add_Line(uint8_t col, uint8_t y1, uint8_t x1, int16_t m, uint8_t y2){

	GradLineStructX2 *KludgePointer;
	
	if (linkNext == 255) { return; }                                        // If we have run out of space to add new lines then fail

	KludgePointer = (GradLineStructX2*)&gradLines[linkNext];
	
	if (linkNext != 0) { KludgePointer->last = linkNext + 1;	};          // If we are pointing to array index[0] then don't update "next"
                                                                            //   index[1] is the first valid entry that can be used.
																			
	linkNext++;                                                             // Update the "Next" to point to the current index (it was pointing to last)
	
	KludgePointer->next = 0;                                                // Insert the values
	KludgePointer->col  = col;
	KludgePointer->y1   = y1;
	KludgePointer->x1   = x1;
	KludgePointer->x1Lo = 0;
	KludgePointer->m    = m;
	KludgePointer->y2   = y2;
}

Which compiles to this

void Add_Line(uint8_t col, uint8_t y1, uint8_t x1, int16_t m, uint8_t y2){
    2a18:	0f 93       	push	r16
    2a1a:	d9 01       	movw	r26, r18

	GradLineStructX2 *KludgePointer;
	
	if (linkNext == 255) { return; }                                        // If we have run out of space to add new lines then fail
    2a1c:	90 91 c7 01 	lds	r25, 0x01C7	; 0x8001c7 <linkNext>
    2a20:	9f 3f       	cpi	r25, 0xFF	; 255
    2a22:	a1 f0       	breq	.+40     	; 0x2a4c <Add_Line+0x34>

	KludgePointer = (GradLineStructX2*)&gradLines[linkNext];
    2a24:	28 e0       	ldi	r18, 0x08	; 8
    2a26:	92 9f       	mul	r25, r18
    2a28:	f0 01       	movw	r30, r0
    2a2a:	11 24       	eor	r1, r1
    2a2c:	e0 50       	subi	r30, 0x00	; 0
    2a2e:	fc 4f       	sbci	r31, 0xFC	; 252
    2a30:	31 e0       	ldi	r19, 0x01	; 1
    2a32:	39 0f       	add	r19, r25
	
	if (linkNext != 0) { KludgePointer->last = linkNext + 1;	};              // If we are pointing to array index[0] then don't update "next"
    2a34:	91 11       	cpse	r25, r1
    2a36:	30 83       	st	Z, r19
                                                                            //   index[1] is the first valid entry that can be used.
																			
	linkNext++;                                                             // Update the "Next" to point to the current index (it was pointing to last)
    2a38:	30 93 c7 01 	sts	0x01C7, r19	; 0x8001c7 <linkNext>
	
	KludgePointer->next = 0;                                           // Insert the values
    2a3c:	10 86       	std	Z+8, r1	; 0x08
	KludgePointer->col  = col;
    2a3e:	81 87       	std	Z+9, r24	; 0x09
	KludgePointer->y1   = y1;
    2a40:	62 87       	std	Z+10, r22	; 0x0a
	KludgePointer->x1   = x1;
    2a42:	44 87       	std	Z+12, r20	; 0x0c
	KludgePointer->x1Lo = 0;
    2a44:	13 86       	std	Z+11, r1	; 0x0b
	KludgePointer->m    = m;
    2a46:	b6 87       	std	Z+14, r27	; 0x0e
    2a48:	a5 87       	std	Z+13, r26	; 0x0d
	KludgePointer->y2   = y2;
    2a4a:	07 87       	std	Z+15, r16	; 0x0f
}
    2a4c:	0f 91       	pop	r16
    2a4e:	08 95       	ret

Now it still has the superfluous PUSH/POP/SUBI - but has gotten down in size by 10%.

But it does look kludgy.

Can a Compiler+ASM guru show me a better way to make the 2nd array access not do the expensive re-calculate of the array