On Sat, Jul 30, 2016 at 08:17:59AM +0000, Bernd Edlinger wrote: > Ok, I the incorrect REG_POINTER is done here: > > cse_main -> reg_scan -> reg_scan_mark_refs -> set_reg_attrs_from_value > > and here I see a bug, because if POINTERS_EXTEND_UNSIGNED > can_be_reg_pointer is only set to false if x is SIGN_EXTEND but not > if x is a SUBREG as in this case. > > So I think that should be fixed this way: > > Index: emit-rtl.c > =================================================================== > --- emit-rtl.c (revision 238891) > +++ emit-rtl.c (working copy) > @@ -1155,7 +1155,7 @@ set_reg_attrs_from_value (rtx reg, rtx x) > || (GET_CODE (x) == SUBREG && subreg_lowpart_p (x))) > { > #if defined(POINTERS_EXTEND_UNSIGNED) > - if (((GET_CODE (x) == SIGN_EXTEND && POINTERS_EXTEND_UNSIGNED) > + if (((GET_CODE (x) != ZERO_EXTEND && POINTERS_EXTEND_UNSIGNED) > || (GET_CODE (x) != SIGN_EXTEND && ! POINTERS_EXTEND_UNSIGNED)) > && !targetm.have_ptr_extend ()) > can_be_reg_pointer = false; > > > What do you think does this look like the right fix?
There also is (from rtl.texi), for paradoxical subregs: """ @item @code{subreg} of @code{reg}s The upper bits are defined when @code{SUBREG_PROMOTED_VAR_P} is true. @code{SUBREG_PROMOTED_UNSIGNED_P} describes what the upper bits hold. Such subregs usually represent local variables, register variables and parameter pseudo variables that have been promoted to a wider mode. """ so you might want to test for these as well. > With this patch the code the reg/f:DI 481 does no longer appear, > and also the invalid combine does no longer happen. Good :-) > However the test case from pr70903 does not get fixed by this. > > But when I look at the dumps, I see again the first incorrect > transformation in cse2 (again cse!): > > (insn 20 19 21 2 (set (reg:DI 94) > (subreg:DI (reg:QI 93) 0)) pr.c:8 50 {*movdi_aarch64} > (expr_list:REG_EQUAL (const_int 255 [0xff]) > (expr_list:REG_DEAD (reg:QI 93) > (nil)))) > > but that is simply wrong, because later optimization passes > expect reg 94 to be 0x000000ff but the upper bits are unspecified, > so that REG_EQUAL should better not exist. Agreed. So where does that come from? > When I looked at cse.c I saw a comment in #if 0, which exactly > describes the problem that we have with paradoxical subreg here: <snip> > I am pretty sure, that this has to be reverted, and that it can > generate less efficient code. > > What do you think? I think this pessimises the generated code too much; there must be a better solution. Segher