saleyn / eixx

Erlang C++ Interface
Apache License 2.0
137 stars 26 forks source link

armv7hl: Error: bad instruction `bsfl [r5],r1` #19

Closed matwey closed 8 years ago

matwey commented 8 years ago

When compiling on armv7hl I have the following:

[  761s] /tmp/ccv1DxyP.s: Assembler messages:
[  761s] /tmp/ccv1DxyP.s:154954: Error: bad instruction `bsfl [r5],r1'
[  769s] Makefile:673: recipe for target 'test_node-test_node.o' failed

Not all platforms havebsf* instructions, but some of them (for instance aarch64) seem to have alternatives.

static __inline__ unsigned long bit_scan_forward(unsigned long v)
{
    unsigned long r;
    __asm__ __volatile__(
        #if (__SIZEOF_LONG__ == 8)
            "bsfq %1, %0": "=r"(r): "rm"(v) );
        #else
            "bsfl %1, %0": "=r"(r): "rm"(v) );
        #endif
    return r;
}

Also, why don't just use things like __builtin_ffsl which will allow compiler to handle proper implementation on all platforms?

matwey commented 8 years ago

Moreover, I am not sure that it makes sense but current implementation doesn't make difference between 0x0 and 0x1 returning 0 in both cases.

matwey commented 8 years ago

Please, look at https://goo.gl/vS3hJT __builtin_ctzl generates same assembler like yours both for gcc and clang, but supports all platforms.

matwey commented 8 years ago

See possible implementation in PR #18

saleyn commented 8 years ago

At the time of writing eixx gcc didn't have these built-in functions, so I had to use assembler. I certainly don't mind switching to build-ins since now it's no longer an issue.

saleyn commented 8 years ago

Actually it looks like builtin_ctzl(0) = 64. Seems like this is currently an issue in the transport_msg, as it's not checking for the UNDEFINED message type, but relies on the fact that bit_scan_forward returns 0 for 0. I believe we could use builtin_ffsl.

matwey commented 8 years ago

__builtin_ctzl(0) is UB:

— Built-in Function: int __builtin_ctz (unsigned int x)

Returns the number of trailing 0-bits in x, starting at the least significant bit position. If x is 0, the result is undefined.

saleyn commented 8 years ago

Can you submit the PR for __builtin_ffsl? Also, to make it deterministic, transport_msg::to_type() should return: m_type == UNDEFINED ? 0 : bit_scan_forward(m_type)