Support for MSP430-GCC

Andrew G

Other Parts Discussed in Thread: MSP430G2553

Hello. Is this the right place to submit feature requests for the new msp430-gcc compiler? If so, here it goes:

When shifting left or right by one, the new compiler relies on functions like __mspabi_srll_1 in libgcc.a instead of the native RRA / RLA instructions. This is slow, and unnecessarily increases the binary size by about 200 bytes in -O2 mode.

Please consider emitting RRA / RLA instructions for shifts by 1.

gcc version 4.9.1 20140707 (prerelease (msp430-14r1-364)) (GNUPro 14r1) (Based on: GCC 4.8 GDB 7.7 Binutils 2.24 Newlib 2.1) (GCC)

over 9 years ago

0 Andrew G over 9 years ago

Intellectual 900 points

More specifically, compiling

bool UART::Busy() const {
  return !(IFG2 & UCA0TXIFG);
}

leads to the following unnecessary bloat (as compiled with -O2 -ffunction-sections --gc-sections). The call to __mspabi_srll_1 can be trivially replaced by RRA r12 in this case.

bool UART::Busy() const {
  return !(IFG2_R & UCA0TXIFG);
    c226:	5c 42 03 00 	mov.b	&0x0003,r12	;0x0003
    c22a:	b0 12 da c2 	call	#49882		;#0xc2da
    c22e:	5c e3       	xor.b	#1,	r12	;r3 As==01
}
    c230:	5c f3       	and.b	#1,	r12	;r3 As==01
    c232:	30 41       	ret

0000c2a2 <__mspabi_srli_15>:
    c2a2:	12 c3       	clrc			
    c2a4:	0c 10       	rrc	r12		;

0000c2a6 <__mspabi_srli_14>:
    c2a6:	12 c3       	clrc			
    c2a8:	0c 10       	rrc	r12		;

0000c2aa <__mspabi_srli_13>:
    c2aa:	12 c3       	clrc			
    c2ac:	0c 10       	rrc	r12		;

0000c2ae <__mspabi_srli_12>:
    c2ae:	12 c3       	clrc			
    c2b0:	0c 10       	rrc	r12		;

0000c2b2 <__mspabi_srli_11>:
    c2b2:	12 c3       	clrc			
    c2b4:	0c 10       	rrc	r12		;

0000c2b6 <__mspabi_srli_10>:
    c2b6:	12 c3       	clrc			
    c2b8:	0c 10       	rrc	r12		;

0000c2ba <__mspabi_srli_9>:
    c2ba:	12 c3       	clrc			
    c2bc:	0c 10       	rrc	r12		;

0000c2be <__mspabi_srli_8>:
    c2be:	12 c3       	clrc			
    c2c0:	0c 10       	rrc	r12		;

0000c2c2 <__mspabi_srli_7>:
    c2c2:	12 c3       	clrc			
    c2c4:	0c 10       	rrc	r12		;

0000c2c6 <__mspabi_srli_6>:
    c2c6:	12 c3       	clrc			
    c2c8:	0c 10       	rrc	r12		;

0000c2ca <__mspabi_srli_5>:
    c2ca:	12 c3       	clrc			
    c2cc:	0c 10       	rrc	r12		;

0000c2ce <__mspabi_srli_4>:
    c2ce:	12 c3       	clrc			
    c2d0:	0c 10       	rrc	r12		;

0000c2d2 <__mspabi_srli_3>:
    c2d2:	12 c3       	clrc			
    c2d4:	0c 10       	rrc	r12		;

0000c2d6 <__mspabi_srli_2>:
    c2d6:	12 c3       	clrc			
    c2d8:	0c 10       	rrc	r12		;

0000c2da <__mspabi_srli_1>:
    c2da:	12 c3       	clrc			
    c2dc:	0c 10       	rrc	r12		;
    c2de:	30 41       	ret			
    c2e0:	3d 53       	add	#-1,	r13	;r3 As==11
    c2e2:	12 c3       	clrc			
    c2e4:	0c 10       	rrc	r12		;

0000c2e6 <__mspabi_srli>:
    c2e6:	0d 93       	cmp	#0,	r13	;r3 As==00
    c2e8:	fb 23       	jnz	$-8      	;abs 0xc2e0
    c2ea:	30 41       	ret			

0000c2ec <__mspabi_srll_15>:
    c2ec:	12 c3       	clrc			
    c2ee:	0d 10       	rrc	r13		;
    c2f0:	0c 10       	rrc	r12		;

0000c2f2 <__mspabi_srll_14>:
    c2f2:	12 c3       	clrc			
    c2f4:	0d 10       	rrc	r13		;
    c2f6:	0c 10       	rrc	r12		;

0000c2f8 <__mspabi_srll_13>:
    c2f8:	12 c3       	clrc			
    c2fa:	0d 10       	rrc	r13		;
    c2fc:	0c 10       	rrc	r12		;

0000c2fe <__mspabi_srll_12>:
    c2fe:	12 c3       	clrc			
    c300:	0d 10       	rrc	r13		;
    c302:	0c 10       	rrc	r12		;

0000c304 <__mspabi_srll_11>:
    c304:	12 c3       	clrc			
    c306:	0d 10       	rrc	r13		;
    c308:	0c 10       	rrc	r12		;

0000c30a <__mspabi_srll_10>:
    c30a:	12 c3       	clrc			
    c30c:	0d 10       	rrc	r13		;
    c30e:	0c 10       	rrc	r12		;

0000c310 <__mspabi_srll_9>:
    c310:	12 c3       	clrc			
    c312:	0d 10       	rrc	r13		;
    c314:	0c 10       	rrc	r12		;

0000c316 <__mspabi_srll_8>:
    c316:	12 c3       	clrc			
    c318:	0d 10       	rrc	r13		;
    c31a:	0c 10       	rrc	r12		;

0000c31c <__mspabi_srll_7>:
    c31c:	12 c3       	clrc			
    c31e:	0d 10       	rrc	r13		;
    c320:	0c 10       	rrc	r12		;

0000c322 <__mspabi_srll_6>:
    c322:	12 c3       	clrc			
    c324:	0d 10       	rrc	r13		;
    c326:	0c 10       	rrc	r12		;

0000c328 <__mspabi_srll_5>:
    c328:	12 c3       	clrc			
    c32a:	0d 10       	rrc	r13		;
    c32c:	0c 10       	rrc	r12		;

0000c32e <__mspabi_srll_4>:
    c32e:	12 c3       	clrc			
    c330:	0d 10       	rrc	r13		;
    c332:	0c 10       	rrc	r12		;

0000c334 <__mspabi_srll_3>:
    c334:	12 c3       	clrc			
    c336:	0d 10       	rrc	r13		;
    c338:	0c 10       	rrc	r12		;

0000c33a <__mspabi_srll_2>:
    c33a:	12 c3       	clrc			
    c33c:	0d 10       	rrc	r13		;
    c33e:	0c 10       	rrc	r12		;

0000c340 <__mspabi_srll_1>:
    c340:	12 c3       	clrc			
    c342:	0d 10       	rrc	r13		;
    c344:	0c 10       	rrc	r12		;
    c346:	30 41       	ret			
    c348:	3e 53       	add	#-1,	r14	;r3 As==11
    c34a:	12 c3       	clrc			
    c34c:	0d 10       	rrc	r13		;
    c34e:	0c 10       	rrc	r12		;

0000c350 <__mspabi_srll>:
    c350:	0e 93       	cmp	#0,	r14	;r3 As==00
    c352:	fa 23       	jnz	$-10     	;abs 0xc348
    c354:	30 41       	ret

0 George Mock over 9 years ago in reply to Andrew G

TI__Guru**** 244600 points

Thank you for letting us know about this issue. I notified the experts at RedHat.

Thanks and regards,

-George

0 DJ Delorie over 9 years ago

Intellectual 390 points

I tried to reproduce this and couldn't, which probably means my test case doesn't match yours exactly. Could you (1) preprocess it, and (2) trim it down to a small stand-alone test case?

0 Andrew G over 9 years ago in reply to DJ Delorie

Intellectual 900 points

I can try to build a small stand-alone test case.

Are you saying that in your test

bool Busy() const {
return !(IFG2 & UCA0TXIFG);
}

avoids a call to __mspabi_srll_1 when compiled with -O2? If so, what does your disassembly look like?

It is possible that my version of gcc is out date. I believe I built gcc from sources about a month ago.

0 Andrew G over 9 years ago in reply to Andrew G

Intellectual 900 points

Simple test:

main.cc

#include <msp430g2553.h>

bool Test() {
  return !(IFG2 & UCA0TXIFG);
}

int main() {
  Test();
}

Compile with

/opt/msp430/bin/msp430-elf-gcc -mmcu=msp430g2553 -O2 -o main.elf main.cc -I/opt/msp430/include

Then disassemble with msp430-objdump -dS main.elf

0000c150 <_Z4Testv>:
    c150:	04 12       	push	r4		
    c152:	04 41       	mov	r1,	r4	
    c154:	5c 42 03 00 	mov.b	&0x0003,r12	
    c158:	b0 12 aa c1 	call	#0xc1aa	
    c15c:	5c e3       	xor.b	#1,	r12	;r3 As==01
    c15e:	5c f3       	and.b	#1,	r12	;r3 As==01
    c160:	34 41       	pop	r4		
    c162:	30 41       	ret			

...	

0000c172 <__mspabi_srli_15>:
    c172:	12 c3       	clrc			
    c174:	0c 10       	rrc	r12		

0000c176 <__mspabi_srli_14>:
    c176:	12 c3       	clrc			
    c178:	0c 10       	rrc	r12		

0000c17a <__mspabi_srli_13>:
    c17a:	12 c3       	clrc			
    c17c:	0c 10       	rrc	r12		

0000c17e <__mspabi_srli_12>:
    c17e:	12 c3       	clrc			
    c180:	0c 10       	rrc	r12		

0000c182 <__mspabi_srli_11>:
    c182:	12 c3       	clrc			
    c184:	0c 10       	rrc	r12		

0000c186 <__mspabi_srli_10>:
    c186:	12 c3       	clrc			
    c188:	0c 10       	rrc	r12		

0000c18a <__mspabi_srli_9>:
    c18a:	12 c3       	clrc			
    c18c:	0c 10       	rrc	r12		

0000c18e <__mspabi_srli_8>:
    c18e:	12 c3       	clrc			
    c190:	0c 10       	rrc	r12		

0000c192 <__mspabi_srli_7>:
    c192:	12 c3       	clrc			
    c194:	0c 10       	rrc	r12		

0000c196 <__mspabi_srli_6>:
    c196:	12 c3       	clrc			
    c198:	0c 10       	rrc	r12		

0000c19a <__mspabi_srli_5>:
    c19a:	12 c3       	clrc			
    c19c:	0c 10       	rrc	r12		

0000c19e <__mspabi_srli_4>:
    c19e:	12 c3       	clrc			
    c1a0:	0c 10       	rrc	r12		

0000c1a2 <__mspabi_srli_3>:
    c1a2:	12 c3       	clrc			
    c1a4:	0c 10       	rrc	r12		

0000c1a6 <__mspabi_srli_2>:
    c1a6:	12 c3       	clrc			
    c1a8:	0c 10       	rrc	r12		

0000c1aa <__mspabi_srli_1>:
    c1aa:	12 c3       	clrc			
    c1ac:	0c 10       	rrc	r12		
    c1ae:	30 41       	ret			

0000c1b0 <.L11>:
    c1b0:	3d 53       	add	#-1,	r13	;r3 As==11
    c1b2:	12 c3       	clrc			
    c1b4:	0c 10       	rrc	r12		

0000c1b6 <__mspabi_srli>:
    c1b6:	0d 93       	tst	r13		
    c1b8:	fb 23       	jnz	$-8      	;abs 0xc1b0
    c1ba:	30 41       	ret			

0000c1bc <__mspabi_srll_15>:
    c1bc:	12 c3       	clrc			
    c1be:	0d 10       	rrc	r13		
    c1c0:	0c 10       	rrc	r12		

0000c1c2 <__mspabi_srll_14>:
    c1c2:	12 c3       	clrc			
    c1c4:	0d 10       	rrc	r13		
    c1c6:	0c 10       	rrc	r12		

0000c1c8 <__mspabi_srll_13>:
    c1c8:	12 c3       	clrc			
    c1ca:	0d 10       	rrc	r13		
    c1cc:	0c 10       	rrc	r12		

0000c1ce <__mspabi_srll_12>:
    c1ce:	12 c3       	clrc			
    c1d0:	0d 10       	rrc	r13		
    c1d2:	0c 10       	rrc	r12		

0000c1d4 <__mspabi_srll_11>:
    c1d4:	12 c3       	clrc			
    c1d6:	0d 10       	rrc	r13		
    c1d8:	0c 10       	rrc	r12		

0000c1da <__mspabi_srll_10>:
    c1da:	12 c3       	clrc			
    c1dc:	0d 10       	rrc	r13		
    c1de:	0c 10       	rrc	r12		

0000c1e0 <__mspabi_srll_9>:
    c1e0:	12 c3       	clrc			
    c1e2:	0d 10       	rrc	r13		
    c1e4:	0c 10       	rrc	r12		

0000c1e6 <__mspabi_srll_8>:
    c1e6:	12 c3       	clrc			
    c1e8:	0d 10       	rrc	r13		
    c1ea:	0c 10       	rrc	r12		

0000c1ec <__mspabi_srll_7>:
    c1ec:	12 c3       	clrc			
    c1ee:	0d 10       	rrc	r13		
    c1f0:	0c 10       	rrc	r12		

0000c1f2 <__mspabi_srll_6>:
    c1f2:	12 c3       	clrc			
    c1f4:	0d 10       	rrc	r13		
    c1f6:	0c 10       	rrc	r12		

0000c1f8 <__mspabi_srll_5>:
    c1f8:	12 c3       	clrc			
    c1fa:	0d 10       	rrc	r13		
    c1fc:	0c 10       	rrc	r12		

0000c1fe <__mspabi_srll_4>:
    c1fe:	12 c3       	clrc			
    c200:	0d 10       	rrc	r13		
    c202:	0c 10       	rrc	r12		

0000c204 <__mspabi_srll_3>:
    c204:	12 c3       	clrc			
    c206:	0d 10       	rrc	r13		
    c208:	0c 10       	rrc	r12		

0000c20a <__mspabi_srll_2>:
    c20a:	12 c3       	clrc			
    c20c:	0d 10       	rrc	r13		
    c20e:	0c 10       	rrc	r12		

0000c210 <__mspabi_srll_1>:
    c210:	12 c3       	clrc			
    c212:	0d 10       	rrc	r13		
    c214:	0c 10       	rrc	r12		
    c216:	30 41       	ret			

0000c218 <.L12>:
    c218:	3e 53       	add	#-1,	r14	;r3 As==11
    c21a:	12 c3       	clrc			
    c21c:	0d 10       	rrc	r13		
    c21e:	0c 10       	rrc	r12		

0000c220 <__mspabi_srll>:
    c220:	0e 93       	tst	r14		
    c222:	fa 23       	jnz	$-10     	;abs 0xc218
    c224:	30 41       	ret

0 DJ Delorie over 9 years ago in reply to Andrew G

Intellectual 390 points

Please add "--save-temps" to your compile command line, and email the *.i file that's produced to dj@redhat.com. I don't have your header files easily accessible here

0 Andrew G over 9 years ago in reply to DJ Delorie

Intellectual 900 points

You don't really need any headers here:

main.cc

#define IFG2          (*((unsigned char*) 0x0003))
#define UCA0TXIFG     0x02

bool Test() {
  return !(IFG2 & UCA0TXIFG);
}

int main() {
  Test();
}

/opt/msp430/bin/msp430-elf-gcc -mmcu=msp430g2553 -O2 -o main.elf main.cc

But I will email the *.i file just in case. Thanks for taking a look.

0 DJ Delorie over 9 years ago in reply to DJ Delorie

Intellectual 390 points

Please try this patch:

Index: msp430.md
===================================================================
RCS file: gcc/config/msp430/msp430.md,v
diff -p -U 5 gcc/config/msp430/msp430.md
--- gcc/config/msp430/msp430.md 28 Jul 2015 04:31:12 -0000
+++ gcc/config/msp430/msp430.md 9 Jan 2016 00:44:28 -0000
@@ -763,10 +763,13 @@
if (msp430x
&& REG_P (operands[0])
&& REG_P (operands[1])
&& CONST_INT_P (operands[2]))
emit_insn (gen_430x_shift_left (operands[0], operands[1], operands[2]));
+ else if (CONST_INT_P (operands[2])
+ && INTVAL (operands[2]) == 1)
+ emit_insn (gen_slli_1 (operands[0], operands[1]));
else
msp430_expand_helper (operands, \"__mspabi_slli\", true);
DONE;
}
)
@@ -832,10 +835,13 @@
if (msp430x
&& REG_P (operands[0])
&& REG_P (operands[1])
&& CONST_INT_P (operands[2]))
emit_insn (gen_430x_arithmetic_shift_right (operands[0], operands[1], operands[2]));
+ else if (CONST_INT_P (operands[2])
+ && INTVAL (operands[2]) == 1)
+ emit_insn (gen_srai_1 (operands[0], operands[1]));
else
msp430_expand_helper (operands, \"__mspabi_srai\", true);
DONE;
}
)
@@ -917,10 +923,13 @@
if (msp430x
&& REG_P (operands[0])
&& REG_P (operands[1])
&& CONST_INT_P (operands[2]))
emit_insn (gen_430x_logical_shift_right (operands[0], operands[1], operands[2]));
+ else if (CONST_INT_P (operands[2])
+ && INTVAL (operands[2]) == 1)
+ emit_insn (gen_srli_1 (operands[0], operands[1]));
else
msp430_expand_helper (operands, \"__mspabi_srli\", true);
DONE;
}
)

0 Andrew G over 9 years ago in reply to DJ Delorie

Intellectual 900 points

Will give it a try, thank you for the quick turnaround!

It may be worth doing this for other small shifts too (e.g. shifting by 2), although in some cases that may actually hurt code size.

I am not familiar with gcc architecture, but I am guessing it is an optimizing compiler, with a way to specify the relative cost of various operations. If so, it may make sense to dial up the cost of shifts (which are inefficient in msp430), causing the compiler to try other approaches. There are a few other ways to compile !(IFG2 & 2).

Thanks again.

0 Andrew G over 9 years ago in reply to DJ Delorie

Intellectual 900 points

Can confirm that the patch works correctly for my application. Thank you for the quick fix!

0 Andrew G over 9 years ago in reply to DJ Delorie

Intellectual 900 points

DJ,

I'd like to make one more GCC feature request:

- To logically shift left by 8, the compiler could output

SWPB + AND 0xFF00

- To shift right by 8 logically:

SWPB + AND 0x00FF

This would benefit MSP430X too, since it is only able to shift efficiently up to 4 positions.

What do you think?

Code Composer Studio™︎

Code Composer Studio forum

Support for MSP430-GCC