[Bug rtl-optimization/50110] New: Endian reversal when adding extzv instruction
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50110
Bug #: 50110
Summary: Endian reversal when adding extzv instruction
Classification: Unclassified
Product: gcc
Version: 4.6.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: rtl-optimization
AssignedTo: unassig...@gcc.gnu.org
ReportedBy: david.me...@icron.com
We are in the process of adding a bitfield extract instruction to the SPARC
Leon processor for a custom embedded product
I've added the following to the sparc.md file
(define_insn "extzvsi"
[(set (match_operand:SI 0 "register_operand" "=r")
(zero_extract:SI (match_operand:SI 1 "register_operand" "r")
(match_operand 2 "const_int_operand" "")
(match_operand 3 "const_int_operand" "")))]
""
"ext\t%1,%2,%3,%0"
)
and GCC will start producing assembly code that uses the new opcode
However on simple test programs the endian seems to be reversed
struct x {
unsigned int a:4;
unsigned int b:2;
unsigned int c:3;
unsigned int d:3;
unsigned int e:20;
};
unsigned int a(struct x * arg)
{ return arg->a; }
unsigned int b(struct x * arg)
{ return arg->b; }
unsigned int c(struct x * arg)
{ return arg->c; }
unsigned int d(struct x * arg)
{ return arg->d; }
unsigned int e(struct x * arg)
{ return arg->e; }
compiled with -Os -S
results in
.file "davidm.c"
.section".text"
.align 4
.global a
.type a, #function
.proc 016
a:
ld [%o0], %o0
jmp %o7+8
srl%o0, 28, %o0
.size a, .-a
.align 4
.global b
.type b, #function
.proc 016
b:
ld [%o0], %o0
jmp %o7+8
ext%o0,2,4,%o0
.size b, .-b
.align 4
.global c
.type c, #function
.proc 016
c:
ld [%o0], %o0
jmp %o7+8
ext%o0,3,6,%o0
.size c, .-c
.align 4
.global d
.type d, #function
.proc 016
d:
ld [%o0], %o0
jmp %o7+8
ext%o0,3,9,%o0
.size d, .-d
.align 4
.global e
.type e, #function
.proc 016
e:
ld [%o0], %g1
sethi %hi(-1048576), %o0
jmp %o7+8
andn %g1, %o0, %o0
.size e, .-e
.ident "GCC: (GNU) 4.6.1"
Just looking at functions a() and b(), a() is using the bitfields in big endian
order as it should for SPARC, but b() seems to be using the bitfields in little
endian order.
[Bug rtl-optimization/50110] Endian reversal when adding extzv instruction
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50110
--- Comment #1 from David Meggy 2011-08-17
19:18:21 UTC ---
When I compile with with -da to dump all the temporary files the endian
reversal seems to happen in the .179r.combine file.
The following block of code is in the .178r.dce file
(insn 6 3 7 2 (set (reg:SI 114)
(mem/s:SI (reg/v/f:SI 111 [ arg ]) [0+0 S4 A32])) davidm.c:13 39
{*movsi_insn}
(expr_list:REG_DEAD (reg/v/f:SI 111 [ arg ])
(nil)))
(insn 7 6 9 2 (set (reg:SI 113)
(lshiftrt:SI (reg:SI 114)
(const_int 26 [0x1a]))) davidm.c:13 258 {lshrsi3}
(expr_list:REG_DEAD (reg:SI 114)
(nil)))
(insn 9 7 10 2 (set (reg:SI 116)
(and:SI (reg:SI 113)
(const_int 3 [0x3]))) davidm.c:13 166 {andsi3}
(expr_list:REG_DEAD (reg:SI 113)
(nil)))
(insn 10 9 15 2 (set (reg:SI 112)
(zero_extend:SI (subreg:QI (reg:SI 116) 3))) davidm.c:13 91
{*zero_extendqisi2_insn}
(expr_list:REG_DEAD (reg:SI 116)
(nil)))
(insn 15 10 18 2 (set (reg/i:SI 24 %i0)
(reg:SI 112)) davidm.c:13 39 {*movsi_insn}
(expr_list:REG_DEAD (reg:SI 112)
(nil)))
which contains a lshrsi3 and andsi3 (not being familar with GCC internals, I
presume this is right shift 26 followed by and of 0x3). Which looks correct
In the .179r.combine file following snippet is present
(insn 6 3 7 2 (set (reg:SI 114)
(mem/s:SI (reg:SI 24 %i0 [ arg ]) [0+0 S4 A32])) davidm.c:13 39
{*movsi_insn}
(expr_list:REG_DEAD (reg:SI 24 %i0 [ arg ])
(nil)))
(note 7 6 9 2 NOTE_INSN_DELETED)
(note 9 7 10 2 NOTE_INSN_DELETED)
(note 10 9 15 2 NOTE_INSN_DELETED)
(insn 15 10 18 2 (set (reg/i:SI 24 %i0)
(zero_extract:SI (reg:SI 114)
(const_int 2 [0x2])
(const_int 4 [0x4]))) davidm.c:13 88 {extzvsi}
(expr_list:REG_DEAD (reg:SI 114)
(nil)))
Which appears that this pass in the compiler swapped the endian as the extzvsi
instruction is now using the constants 2 and 4, which would represent a little
endian bitfield structure.
[Bug rtl-optimization/50110] Endian reversal when adding extzv instruction
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50110 --- Comment #3 from David Meggy 2011-08-17 19:37:48 UTC --- Andrew, are you referring to an issue with the define_insn macro I created? or the GCC zero-extract generic code? I've taken a look at http://gcc.gnu.org/onlinedocs/gccint/Standard-Names.html#Standard-Names (scrolling down to extzv and extv), and I believe that I'm using the extraction as intended to be used
[Bug rtl-optimization/50110] Endian reversal when adding extzv instruction
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50110 --- Comment #4 from David Meggy 2011-08-17 23:15:26 UTC --- >From rtl.def /* Reference to a signed bit-field of specified size and position. Operand 0 is the memory unit (usually SImode or QImode) which contains the field's first bit. Operand 1 is the width, in bits. Operand 2 is the number of bits in the memory unit before the first bit of this field. If BITS_BIG_ENDIAN is defined, the first bit is the msb and operand 2 counts from the msb of the memory unit. Otherwise, the first bit is the lsb and operand 2 counts from the lsb of the memory unit. This kind of expression can not appear as an lvalue in RTL. */ DEF_RTL_EXPR(SIGN_EXTRACT, "sign_extract", "eee", RTX_BITFIELD_OPS) /* Similar for unsigned bit-field. But note! This kind of expression _can_ appear as an lvalue. */ DEF_RTL_EXPR(ZERO_EXTRACT, "zero_extract", "eee", RTX_BITFIELD_OPS) It appears that this behaviour is intended. To me it seems logical that the extract instruction is really just a "right shift", followed by an "and", where the "and" uses a mask based on the width of the bitfield. To count from the MSB seems counter-intuitive to me. If my understanding is correct this code comment should appear in http://gcc.gnu.org/onlinedocs/gccint/Standard-Names.html#Standard-Names under the extzv and extv headings. I can fix up my define_insn to do the conversion in our implementation.
[Bug rtl-optimization/50110] Endian reversal when adding extzv instruction
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50110 David Meggy changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||INVALID --- Comment #5 from David Meggy 2012-01-10 19:16:15 UTC --- Let me sum up this bug. It boiled down to a misunderstanding of how bit field extract works on a big endian processor (I don't know if anyone else has implemented this on a big endian processor). Since we have implemented the starting bit in our hardware from the least significant bit, I've just modified the output instruction to change the start bit to the least significant bit. (define_insn "extzvsi" [(set (match_operand:SI 0 "register_operand" "=r") (zero_extract:SI (match_operand:SI 1 "register_operand" "r") (match_operand 2 "const_int_operand" "") (match_operand 3 "const_int_operand" "")))] "" "ext\t%1,%2,32 - %2 - %3,%0" ) This works quite well for us, and is giving us the code space savings we were looking for, an ~1% code space reduction. Not bad for a tiny GCC patch, tiny binutils patch, and a minor CPU opcode addition. This bug can be closed. Marking as RESOLVED - INVALID, as it isn't a real bug, but a documentation misunderstanding
[Bug c/49368] New: __builtin_constant_p is unable to determine if a union is constant
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49368
Summary: __builtin_constant_p is unable to determine if a union
is constant
Product: gcc
Version: 4.5.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
AssignedTo: unassig...@gcc.gnu.org
ReportedBy: david.me...@icron.com
In the following code GCC seems to be unable to determine that a const union is
actually const when passed as an argument to __builtin_constant_p(). If I use
a struct instead, then it seems to work, so there is only an issue when using a
union.
This is tested on GCC 4.5.1 & GCC 4.1.2
$ cat dummy.c
#include
union u {
int a;
int b;
};
#define DO_WORK(_arg_) \
do {\
if (__builtin_constant_p(_arg_))\
{ \
printf("Do optimized code\n"); \
} \
else\
{ \
printf("Do slow code\n"); \
} \
} while (0 == 1)
int main()
{
const union u x = { .a = 0 } ;
DO_WORK(x.b);
return 0;
}
$ gcc -Wall -Os dummy.c
$ ./a.out
Do slow code
[Bug c/49368] __builtin_constant_p is unable to determine if a union is constant
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49368 --- Comment #2 from David Meggy 2011-06-13 16:03:01 UTC --- Both those versions are newer than what I'm using. Looks like time to upgrade Thanks for looking into this
