Re: [avr-gcc-list] Optimisation mixes up registers in inline assembly

[Michael Kwasnicki]

Hello David.
Thanks for the explanation.
I intentionally reduced the code to not bother you with details that didn't 
help with pointing out my issue.
So as a conclusion it was my misconception of how inline assembly works.
In the full code I not only count 8 bits and count some bytes but also load 
each byte, shift each bit out and change the actual duty cycle according to the 
bit that lands in the carry bit.
I am new to inline assembly so the first steps are not always useful but 
playful and educational. So I learn more from the inner workings of AVR MCUs.

Cheers,

Michael K.




> Am 24.07.2019 um 23:00 schrieb David Brown <address@hidden>:
> 
> On 24/07/2019 21:41, Michael Kwasnicki wrote:
>> So the inputs to inline assembly are by value and not by reference, as I 
>> thought. Right?
> 
> Correct.  The inputs are used to initialise the registers.
> 
>> The correlation between my named registers and those C variables was pure 
>> coincidence.
> 
> No, it is pure optimisation, not coincidence.  And of course you don't 
> necessarily get registers for variables in C - certainly not for const values.
> 
>> And by making the named registers both - input and output - it creates a 
>> reference to the original C variable.
> 
> Yes, roughly.
> 
> The big question here is why you are using assembly at all.  Why not just 
> write in C?
> 
> void test2(void) {
>    uint8_t size = 42;
>    const uint8_t start = _BV(WGM02) |  _BV(CS00);
>    const uint8_t stop = 0x00;
>    TCCR0A = _BV(COM0B1) | _BV(WGM01) | _BV(WGM00);
>    OCR0A = 9;                              // TOP
>    const uint8_t zero = 1;                 // 0 < i <= TOP
>    const uint8_t one = 7;                        // 0 < i <= TOP
> 
>    OCR0B = one;
>    TCCR0B = start;
> 
>    while (size--) {
>        for (uint8_t bits = 0; bits < 8; bits++) {
>            OCR0B = one;
>        }
>        OCR0B = zero;
>    }
>    TCCR0B = stop;
> }
> 
> It is simpler, easier to understand, much easier to get right, and gives code 
> that is marginally smaller and faster than your hand-written assembly.  (It 
> is even better with -O2, which is the optimisation level I recommend as 
> usually giving smaller and faster code than -Os.)
> 
> mvh.,
> 
> David
> 
> 
> 
>> Regards,
>> Michael K.
>>> Am 24.07.2019 um 19:17 schrieb Joseph C. Sible <address@hidden 
>>> <mailto:address@hidden>>:
>>> 
>>> Since you intend to modify the inputs, you need to declare them as outputs 
>>> as well (with the + constraint). If you don't do this, GCC is allowed to 
>>> assume you don't write to them, and can share any that have the same values.
>>> 
>>> Joseph C. Sible
>>> 
>>> 
>>> On Wed, Jul 24, 2019, 13:04 Michael Kwasnicki <address@hidden 
>>> <mailto:address@hidden>> wrote:
>>> 
>>>    Hello dear list members,
>>> 
>>>    I am new to this list because I need someone to discuss a
>>>    potential compiler bug.
>>> 
>>> 
>>>    I try to mix inline assembly in C code. In the assembly I use
>>>    passed in variables which are named:
>>> 
>>>    _https://godbolt.org/z/w8wuJy_
>>>    _
>>>    _
>>>    The register mapping is:
>>>    [rZero] -> R20
>>>    [rOne] -> R21
>>>    [rStart] -> R18
>>>    [rStop] -> R19
>>>    [rSize] -> R22
>>>    [bits] -> R19
>>> 
>>>    If I look at the output assembly I can see the compiler did some
>>>    optimisation.
>>>    R19 is shared between [bits] and [rStop].
>>>    In this case this is okay as the bit count reaches zero at the end
>>>    and thus [bits] and [rStop] have the same value then.
>>> 
>>> 
>>>    But when I change the initial value of `bitcount` in C to 7 all
>>>    hell breaks loose.
>>> 
>>>    https://godbolt.org/z/F7rrB_
>>> 
>>>    The register mapping is:
>>>    [rZero] -> R20
>>>    [rOne] -> R18
>>>    [rStart] -> R19
>>>    [rStop] -> R21
>>>    [rSize] -> R22
>>>    [bits] -> R18
>>> 
>>>    This time the compiler optimisation corrupted the program.
>>>    R18 is shared between [rOne] and [bits]. [rOne] was supposed to be
>>>    a constant that is cached in a register. But [bits] gets decremented.
>>>    Thus the value passed to OCR0B varies and is not constant any more
>>>    changing the logic of the inline assembly.
>>> 
>>>    I am not experienced with inline assembly so I might have missed
>>>    something but my general expectation for assembly is that if I
>>>    name things differently, they are different things (registers in
>>>    this case).
>>> 
>>>    The issue does not show up with -O0 but anything above.
>>>    Locally I am running avr-gcc (GCC) 9.1.0. Not sure what the exact
>>>    version at godbolt is.
>>> 
>>>    Cheers,
>>> 
>>>    Michael
>>>    _______________________________________________
>>>    AVR-GCC-list mailing list
>>>    address@hidden <mailto:address@hidden>
>>>    https://lists.nongnu.org/mailman/listinfo/avr-gcc-list
>>> 
>> _______________________________________________
>> AVR-GCC-list mailing list
>> address@hidden
>> https://lists.nongnu.org/mailman/listinfo/avr-gcc-list