- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Hi,
It appears that there is a bug in this version of the nios2 compiler at higher optimisation levels. $ nios2-elf-gcc --version nios2-elf-gcc (Altera 11.1sp2 Build 259) 4.1.2 Copyright (C) 2006 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. I can isolate to a small source code as follows. # include <stdint.h> uint32_t makeMask ( const uint32_t l ) { uint32_t mask; if ( l < 32u ) { static const uint32_t one = 1u; const unsigned idx = 32u - l; uint32_t mask = one; mask <<= idx; mask -= one; } else { mask = 31u; } return mask; } Running gcc with these options (highest level of optimisation and producing an assembler listing) appears to indicate that there is a bug in the nios2 compiler. nios2-elf-gcc -c -O3 -S test.c .file "test.c" .section .text .align 2 .global makeMask .type makeMask, @function makeMask: movi r2, 31 ret .size makeMask, .-makeMask .ident "GCC: (Altera 11.1sp2 Build 259) 4.1.2"Link Copied
3 Replies
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
The problem is with your code, not the compiler.
You define mask in two different scopes. All the bit manipulation you do in the first branch of the if-statement is on your second definition of mask. Unfortunately you don't return that one, but the one defined in the outer scope. So basically you're saying to the compiler, return an uninitialised number or 31. In this case the result is undefined, so the compiler is probably free to do what it likes. It likes 31. To fix your code, remove type definition from the first assignment to mask.- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Thanks dabuk for your correction. You are absolutely correct.
BTW, while consuming BW, there was in fact a funny behaviour remaining in this code after it is corrected as debuk suggests (which in fact is the one I was originally pursuing). Do you see it? It resulted from a more subtle programming error (unfortunately still my fault STS). The following is a very bad idea in portable C code. uint32_t mask; if ( l < 32u ) { static const uint32_t one = 1u; const unsigned idx = 32u - l; uint32_t mask = one; mask <<= idx; mask -= one; } This one is correct: uint32_t mask; if ( l > 0 && l < 32u ) { static const uint32_t one = 1u; const unsigned idx = 32u - l; uint32_t mask = one; mask <<= idx; mask -= one; } The problem in the first version is that the C standard says that the result of a left shift is undefined if we specify a shift of more bits than are in the word, and 32 is in fact too many. The nios2 RTL for the SLL instruction is as follows. rC ← rA << (rB4..0) Notice that it uses only 5 bits from the register, shoot self in foot. Ouch- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
You'll also find that the x86 cpu ignores all the high bits of a shift count.
Oh definitions like:static const uint32_t one = 1u;
are just completely pointless. It is much clearer if you just write: uint32_t makeMask(uint32_t count)
{
return count >= 31 ? 0xffffffffu : (1u << count) - 1;
}
All the temporary variables just make it harder to read. (That isn't quite your function, but is closer to the one you wanted!)
Reply
Topic Options
- Subscribe to RSS Feed
- Mark Topic as New
- Mark Topic as Read
- Float this Topic for Current User
- Bookmark
- Subscribe
- Printer Friendly Page