• State Resolved
  • Locked Locked
  • Replies 9 replies
  • Answers 2 answers
  • Subscribers 81 subscribers
  • Views 1421 views
  • Users 0 members are here
Support feedback
Options
Options
Related

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.

MSP430-GCC-OPENSOURCE: Inline ASM constraints weirdness and constraints for the different addressing modes

Thilo Schulz
Prodigy 230 points

Part Number: MSP430-GCC-OPENSOURCE

Hi,

some register constraints do not work as documented at https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html

Consider the following example:

static uint16_t swapvals[8];

uint8_t some_function(int index, uint8_t value)
{
	__asm__ __volatile__
	(
		"mov.b %[val], 1(%[dst])\n"
		"swpb 0(%[dst])\n"
		"mov.b 1(%[dst]), %[val]\n"
		: [val] "+g" (value), [dst] "+o" (swapvals[index])
	);
	
	return value;
}

This function is supposed to implement an interrupt-safe method to read and change an 8 bit value atomically. First, Parameter "value" is written to the high byte of swapvals[index], afterwards the swap is performed on the memory Operand. The new value is now in effect in the low byte, while the old remains stored in the high Byte and can be retrieved.

This is why I chose "+o" as Output Operand constraint which according to GCC doc is:

‘o’
A memory operand is allowed, but only if the address is offsettable.
This means that adding a small integer (actually, the width in bytes of the Operand,
as determined by its machine mode) may be added to the address and the result is
also a valid memory address. 
For example, an address which is constant is offsettable; so is an address that is the sum of a Register
and a constant (as long as a slightly larger constant is also within the range of
address-offsets supported by the machine); but an autoincrement or autodecrement address
is not offsettable. More complicated indirect/indexed addresses may or may not be offsettable
depending on the other addressing modes that the machine supports.

However, on -O0 this generates the following (Code by which gcc loads R13 with the value of &swapvals[index] has been omitted):

mov.b R12, 1(@R13)
swpb 0(@R13)
mov.b 1(@R13), R12

On -O3 it gets even weirder:

mov.b R12, 1(swapvals(R13))
swpb 0(swapvals(R13))
mov.b 1(swapvals(R13)), R12

wth??

@ addressing mode is clearly not offsettable, not mentioning the fact that a Destination operand does not support @ addressing mode anyway.

But what I wanted was:

mov.b R12, 1(R13)
swpb 0(R13)
mov.b 1(R13), R12

Now I realize, the Definition for 'o' constraint would also allow for & addressing mode.. had I used swapvals[0] as Input Operand, it would have generated this:

mov.b R12, 1(&swapvals)
swpb 0(&swapvals)
mov.b 1(&swapvals), R12

So currently, the only way to achieve what I want is using a pointer as pure Input Operand with These constraints:

: [val] "+g" (value)
: [dst] "r" (&swapvals[index])

However, gcc does not notice then that swapvals[index] is clobbered and having to add a "memory" clobber seems excessive to me.

Is there any reason, why we cannot introduce machine-specific constraint characters so one has direct control over the addressing mode that is used for each Input Operand?

  • 0
    Expert 2410 points

    The thing is that constraints don't have anything to with the output assembler template. They're merely used for optimization passes to be able to optimize around the opaque code in the asm directive. So using the "o" constraint just tells GCC that swapvals[index] is a memory address which could validly have an offset, not that the MSP430 backend must generate assembly code which can allow an offset to be appended to it.

    You have to do something like this:

    uint8_t some_function(int index, uint8_t value)
{
    __asm__ __volatile__
    (
        "mov.b %[val], %[hidst]\n"
        "\tswpb %[dst]\n"
        "\tmov.b %[hidst], %[val]\n"
        : [val] "+g" (value), [dst] "+o" (swapvals[index]),
          [hidst] "+o" (*((unsigned char *)(&swapvals[index])+1))
    );
    
    return value;
}

    Since you want to get at byte values of an array of 16-bit values, it is a bit messy.

    Hope this helps,

  • 0
    Guru 26515 points

    This seems like the sort of thing that should be done in straight up assembly code.

  • 0 in reply to David Schultz
    Guru 26515 points

    If you insist on using the inline assembler, make the function a little more general purpose and remove the need to index into the array:

    uint8_t atomic(int *p, uint8_t val)
{
  uint8_t ret;

  asm volatile("mov.b %2,1(%1)\n\t"
   0:   cc 4d 01 00     mov.b   r13,    1(r12)  ;
   4:   ac 10           swpb    @r12            ;
   6:   5c 4c 01 00     mov.b   1(r12), r12     ;

0000000a <.LVL1>:
               "swpb @%1\n\t"
               "mov.b 1(%1),%0\n"
               : "=r" (ret)
               : "r" (p), "r" (val)
               : );
  return ret;
   a:   30 41           ret

  • 0 in reply to Jozef_Lawrynowicz
    Prodigy 230 points

    > So using the "o" constraint just tells GCC that swapvals[index] is a memory address which could validly have an offset, not that the MSP430 backend must generate assembly code which can allow an offset to be appended to it.

    Doing some experimentation on x86 gcc I have come to realize you are quite Right.

    Please have a look at gcc/config/msp430/msp430.c

    It is here where operands are printed out.  There are also several undocumented modifiers controlling how operands are printed.

    For instance, you can write operands like "%B[val]" which will result in 2(r13)

    This scheme seems to have some popularity among other platforms as well. In fact, I know it from avr-gcc where it is documented here:

    https://www.nongnu.org/avr-libc/user-manual/inline_asm.html

    And for some weird reason which I suspect is a bug, "%C[val]" yields 3(r13) instead of 4(r13) which could be abused to do what I want like this:

    static uint16_t swapvals[8];

uint8_t some_function(int index, uint8_t value)
{
    __asm__ __volatile__
    (
        "mov.b %[val], %C[dst]\n"
        "swpb %B[dst]\n"
        "mov.b %C[dst], %[val]\n"
        : [val] "+g" (value), [dst] "+m" (swapvals[index-1])
    );
    
    return value;
}

    Notice the index in swapvals being decremented beforehand (resulting in 1 additional instruction in the gcc generated asm() boilerplate)

    I guess it would be trivial to introduce another machine specific special character that would just add an offset of 1(r13) here.

  • 0 in reply to David Schultz
    Prodigy 230 points

    In my original implementation the inline Assembly part is naturally embedded in a do {} while(0) macro. I have Chosen to Keep the example simpler here.

    I really do want to use inline Assembly here as the call Overhead for a three instruction function seems unnecessary and ugly.

  • 0 in reply to Thilo Schulz
    Guru 26515 points

    GCC has this annoying habit of inlining short functions like this to avoid the call/return overhead. So I see no penalty in keeping this in function form.

  • 0 in reply to Thilo Schulz
    Expert 2410 points

    Thilo Schulz said:
    And for some weird reason which I suspect is a bug, "%C[val]" yields 3(r13) instead of 4(r13) which could be abused to do what I want like this:

    Nice spot, it looks like %C and %D have been implemented incorrectly when used on a memory reference. The documentation at the top of the function says:

    /* A   low 16-bits of int/lower of register pair
      B   high 16-bits of int/higher of register pair
      C   bits 32-47 of a 64-bit value/reg 3 of a DImode value
      D   bits 48-63 of a 64-bit value/reg 4 of a DImode value

    But actually %C offsets the memory reference by 3 bytes (starting from bit 24), and %D offsets by 4 bytes.

    However, these operand selectors should work correctly when the value is stored directly in registers (in which case %C will add 2 to the register number) or constant values.

    We'll get this behaviour cleaned up and documented in the next release. And we'll look to add operand selectors for single byte offsets.

    Regards,

  • 0 in reply to Jozef_Lawrynowicz
    Expert 2410 points

    An ticket for this issue has been filed as MSPGCC-236, it can be tracked here: cqweb.ext.ti.com/.../MSPGCC-236

  • 0 in reply to Jozef_Lawrynowicz
    TI__Guru 71216 points

    Thank you Jozef for jumping in on this and helping our customer.

    Thilo, I'm going to mark this issue resolved since Jozef is the expert on this topic and can assist you going forward.

**Attention** This is a public forum