https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88739
--- Comment #32 from Richard Biener <rguenth at gcc dot gnu.org> --- (In reply to rguent...@suse.de from comment #31) > On Wed, 9 Jan 2019, wilco at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88739 > > > > --- Comment #29 from Wilco <wilco at gcc dot gnu.org> --- > > (In reply to Richard Biener from comment #26) > > > > > Did anybody test the patch? Testing on x86_64 will be quite pointless... > > > > Well that generates _18 = BIT_FIELD_REF <_2, 16, 14>; and becomes: > > > > ubfx x1, x20, 2, 16 > > > > This extracts bits 2-17 of the 30-bit value instead of bits 14-29. The > > issue is > > that we're using a bitfield reference on a value that is claimed not to be a > > bitfield in comment 6. So I can't see how using BIT_FIELD_REF could ever > > work > > correctly. > > So that's because TYPE_PRECISION != GET_MODE_PRECISION and the > BIT_FIELD_REF expansion counting from GET_MODE_PRECISION I suppose. > > Thus there is a RTL expansion side of the bug after all? > > The "fixed" RTL is > > (insn 6 5 7 (set (reg:SI 95) > (lshiftrt:SI (reg/v:SI 94 [ ulAddr ]) > (const_int 2 [0x2]))) "t.c":42:48 -1 > (nil)) > > (insn 7 6 8 (set (reg:SI 96) > (and:SI (reg:SI 95) > (const_int 1073741823 [0x3fffffff]))) "t.c":42:48 -1 > (nil)) > > (insn 8 7 9 (set (subreg:DI (reg:HI 97) 0) > (zero_extract:DI (subreg:DI (reg:SI 96) 0) > (const_int 16 [0x10]) > (const_int 2 [0x2]))) "t.c":44:8 -1 > (nil)) > > so the 30bit value is in reg:SI 96 (the :30 cast causes the > and with 0x3fffffff) but then the zero_extract we generate > is bogus. > > So maybe the :30 cast should have been a shift for BYTES_BIG_ENDIAN? > > We might be able to work around this by optimization on GIMPLE, > combining > > _1 = ulAddr_3(D) >> 2; > _2 = (<unnamed-unsigned:30>) _1; > _6 = BIT_FIELD_REF <_2, 16, 14>; > > as far as eliminating at least the non-mode precision type... > > Of course that would just work around the underlying RTL expansion > bug? > > Note we can end up with things like > > _2 = (<unnamed-unsigned:35) _1; > _3 = (<unnamed-unsigned:35) _4; > _5 = _2 + 3; > > as well so shifting at the conversion might not be the correct > answer (but instead BIT_FIELD_REF expansion needs to be fixed). > > Alternatively we could declare it invalid GIMPLE and require > BIT_FIELD_REF positions to be always relative to the mode > (but then I'd rather disallow BIT_FIELD_REF on non-mode > precision entities...). > > Sth like the following might fix the RTL expansion issue > which then generates > > Test_func: > ubfx x0, x0, 2, 16 > cmp w0, 1 > bne .L6 > mov w0, 0 > > and just > > (insn 6 5 7 (set (reg:SI 95) > (lshiftrt:SI (reg/v:SI 94 [ ulAddr ]) > (const_int 2 [0x2]))) "t.c":42:48 -1 > (nil)) > > (insn 7 6 8 (set (reg:SI 96) > (and:SI (reg:SI 95) > (const_int 1073741823 [0x3fffffff]))) "t.c":42:48 -1 > (nil)) > > (insn 8 7 9 (set (reg:SI 97) > (zero_extend:SI (subreg:HI (reg:SI 96) 2))) "t.c":44:8 -1 > (nil)) > > Index: gcc/expr.c > =================================================================== > --- gcc/expr.c (revision 267553) > +++ gcc/expr.c (working copy) > @@ -10562,6 +10562,15 @@ expand_expr_real_1 (tree exp, rtx target > infinitely recurse. */ > gcc_assert (tem != exp); > > + /* When extracting from non-mode bitsize entities adjust the > + bit position for BYTES_BIG_ENDIAN. */ > + if (INTEGRAL_TYPE_P (TREE_TYPE (tem)) > + && (TYPE_PRECISION (TREE_TYPE (tem)) > + < GET_MODE_BITSIZE (as_a <scalar_int_mode> (TYPE_MODE > (TREE_TYPE (tem))))) > + && BYTES_BIG_ENDIAN) > + bitpos += (GET_MODE_BITSIZE (as_a <scalar_int_mode> (TYPE_MODE > (TREE_TYPE (tem)))) > + - TYPE_PRECISION (TREE_TYPE (tem))); > + > /* If TEM's type is a union of variable size, pass TARGET to the > inner > computation, since it will need a temporary and TARGET is known > to have to do. This occurs in unchecked conversion in Ada. */ Btw, this needs to be amended for WORDS_BIG_ENDIAN of course. I guess we might even run into the case that such BIT_FIELD_REF references a non-contiguous set of bits... (that's also true for BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN I guess).
