JHinkle
JHinkle

Reputation: 202

gcc ARM produces incorrect code - how to correct

gcc ARM for STM32F407 micro

The following function is used as a sanity check in FreeRtosTCP

UBaseType_t bIsValidNetworkDescriptor( const NetworkBufferDescriptor_t * pxDesc )
{

    uint32_t offset = ( uint32_t ) ( ((const char *)pxDesc) - ((const char *)xNetworkBuffers) );

    if( ( offset >= (uint32_t)(sizeof( xNetworkBuffers )) ) || ( ( offset % sizeof( xNetworkBuffers[0] ) ) != 0 ) )
        return pdFALSE;

    return (UBaseType_t) (pxDesc - xNetworkBuffers) + 1;
}

The line in question is ---> offset >= (uint32_t)(sizeof( xNetworkBuffers ))

gcc produces a bhi instruction after the cmp instead of a bhs.

If tries casting both as shown in the code above but nothing seems to get the bhs instruction to be used.

Any help appreciated.

Thanks.

Joe

Upvotes: 1

Views: 164

Answers (2)

Florian Weimer
Florian Weimer

Reputation: 33747

I think the code generated by GCC is correct, technically speaking. offset cannot be larger than INT_MAX, because this is the maximum value representable in ptrdiff_t on this architecture.

You can compute the difference like this:

    uintptr_t offset = (uintptr_t)pxDesc - (uintptr_t)xNetworkBuffers;

This is still implementation-defined, but it will avoid the overflow problem.

Upvotes: 1

Rajnesh
Rajnesh

Reputation: 923

Well knowing the exact size of the xNetworkBuffers array compiler can simply optimize it. Being curious I gave it a try. Following is the code with little modifications and the asm output and the explanation:

#include <stdint.h>

typedef struct abc {
    char data[10];
}NetworkBufferDescriptor_t;
NetworkBufferDescriptor_t xNetworkBuffers[5];

int bIsValidNetworkDescriptor( const NetworkBufferDescriptor_t * pxDesc )
{

    uint32_t offset = ( uint32_t ) ( ((const char *)pxDesc) - ((const char *)xNetworkBuffers) );

    if( ( offset >= (uint32_t)(sizeof( xNetworkBuffers )) ) || ( ( offset % sizeof( xNetworkBuffers[0] ) ) != 0 ) )
        return 0;

    return (int) (pxDesc - xNetworkBuffers) + 1;
}

and the asm output is:

bIsValidNetworkDescriptor:
    @ Function supports interworking.
    @ args = 0, pretend = 0, frame = 16
    @ frame_needed = 1, uses_anonymous_args = 0
    @ link register save eliminated.
    str fp, [sp, #-4]!
    add fp, sp, #0
    sub sp, sp, #20
    str r0, [fp, #-16]
    ldr r3, [fp, #-16]
    ldr r2, .L5
  sub r3, r3, r2
  str r3, [fp, #-8]
  ldr r3, [fp, #-8]
  cmp r3, #49
  bhi .L2
    ldr r1, [fp, #-8]
    ldr r3, .L5+4
    umull   r2, r3, r1, r3
    lsr r2, r3, #3
    mov r3, r2
    lsl r3, r3, #2
    add r3, r3, r2
    lsl r3, r3, #1
    sub r2, r1, r3
    cmp r2, #0
    beq .L3
.L2:
    mov r3, #0
    b   .L4
.L3:
    ldr r3, [fp, #-16]
    ldr r2, .L5
    sub r3, r3, r2
    asr r2, r3, #1
    mov r3, r2
    lsl r3, r3, #1
    add r3, r3, r2
    lsl r1, r3, #4
    add r3, r3, r1
    lsl r1, r3, #8
    add r3, r3, r1
    lsl r1, r3, #16
    add r3, r3, r1
    lsl r3, r3, #2
    add r3, r3, r2
    add r3, r3, #1
.L4:
    mov r0, r3
    add sp, fp, #0
    @ sp needed
    ldr fp, [sp], #4
    bx  lr
.L6:
    .align  2
.L5:

In the block quoted asm code you can see that it is comparing with 49 not 50 (which is the actual size of xNetworkBuffers) so the conclusion I got is

offset >= (uint32_t)(sizeof( xNetworkBuffers ))

is also equal to

offset > (uint32_t)(sizeof( xNetworkBuffers ) - 1) )

and in that case compiler can use BHI producing the same results

Upvotes: 2

Related Questions