This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Support for MSP430-GCC

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)

  • 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			
    

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

    Thanks and regards,

    -George

  • 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?
  • 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.
  • 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

  • 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
  • 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.

  • 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;
    }
    )
  • 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.
  • Can confirm that the patch works correctly for my application. Thank you for the quick fix!

  • 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?