Hi Jeff, On 08/01/16 19:54, Jeff Law wrote: > Looks like you've probably nailed it. It'll be interesting see if > there's any fallout (though our RTL optimizer testing is pretty weak, so > even if there were, I doubt we'd catch it). >
If there is, it will probably a performance regression... Anyway I'd say these two patches do just disable actually wrong transformations. So here are both patches as separate diffs with your suggestion for the comment in cse_insn. I believe that on x86_64 both patches do not change a single bit. However I think there are more paradoxical subregs generated all over, but the aarch64 insv code pattern did trigger more hidden bugs than any other port. It is certainly unfortunate that the major source of paradoxical subreg is in a target-dependent code path :( Please apologize that I am not able to reduce/finalize the aarch64 test case at this time, as I usually only work with arm and intel targets, but I made an exception here, because a bug like that may affect all targets sooner or later. Boot-strap and reg-testing on x86_64-linux-gnu. Plus aarch64 bootstrap and isl-testing by Andreas. Is it OK for trunk? Thanks Bernd.
2016-08-01 Bernd Edlinger <bernd.edlin...@hotmail.de> PR rtl-optimization/70903 * cse.c (cse_insn): If DEST is a paradoxical SUBREG, don't record DEST. Index: gcc/cse.c =================================================================== --- gcc/cse.c (revision 238915) +++ gcc/cse.c (working copy) @@ -5898,15 +5898,7 @@ cse_insn (rtx_insn *insn) || GET_MODE (dest) == BLKmode /* If we didn't put a REG_EQUAL value or a source into the hash table, there is no point is recording DEST. */ - || sets[i].src_elt == 0 - /* If DEST is a paradoxical SUBREG and SRC is a ZERO_EXTEND - or SIGN_EXTEND, don't record DEST since it can cause - some tracking to be wrong. - - ??? Think about this more later. */ - || (paradoxical_subreg_p (dest) - && (GET_CODE (sets[i].src) == SIGN_EXTEND - || GET_CODE (sets[i].src) == ZERO_EXTEND))) + || sets[i].src_elt == 0) continue; /* STRICT_LOW_PART isn't part of the value BEING set, @@ -5925,6 +5917,11 @@ cse_insn (rtx_insn *insn) sets[i].dest_hash = HASH (dest, GET_MODE (dest)); } + /* If DEST is a paradoxical SUBREG, don't record DEST since the bits + outside the mode of GET_MODE (SUBREG_REG (dest)) are undefined. */ + if (paradoxical_subreg_p (dest)) + continue; + elt = insert (dest, sets[i].src_elt, sets[i].dest_hash, GET_MODE (dest));
2016-08-01 Bernd Edlinger <bernd.edlin...@hotmail.de> PR rtl-optimization/71779 * emit-rtl.c (set_reg_attrs_from_value): Only propagate REG_POINTER, if the value was sign-extended according to POINTERS_EXTEND_UNSIGNED or if it was truncated. Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c (revision 238915) +++ gcc/emit-rtl.c (working copy) @@ -1156,7 +1156,11 @@ set_reg_attrs_from_value (rtx reg, rtx x) { #if defined(POINTERS_EXTEND_UNSIGNED) if (((GET_CODE (x) == SIGN_EXTEND && POINTERS_EXTEND_UNSIGNED) - || (GET_CODE (x) != SIGN_EXTEND && ! POINTERS_EXTEND_UNSIGNED)) + || (GET_CODE (x) == ZERO_EXTEND && ! POINTERS_EXTEND_UNSIGNED) + || (paradoxical_subreg_p (x) + && ! (SUBREG_PROMOTED_VAR_P (x) + && SUBREG_CHECK_PROMOTED_SIGN (x, + POINTERS_EXTEND_UNSIGNED)))) && !targetm.have_ptr_extend ()) can_be_reg_pointer = false; #endif