Il 17/12/2012 22:33, Jakub Jelinek ha scritto: > Hi! > > If gen_lowpart_if_possible returns NULL, the default > rtl_hooks.gen_lowpart_no_emit hook returns the original value, which is not > of the desired mode. I bet in most passes for real insns such rtx is then > meant to fail recog and thrown away, but for DEBUG_INSN modification that > doesn't work, since there is no verification (but also e.g. any kind of > SUBREG is fine). So we can e.g. end up with a (plus:SI (mem:DI ...) (mem:SI > ...)) > or similar and then various passes (in this testcase on s390x reload) can be > very upset about that.
Makes sense, and it could even be a wrong-code bug for this simplification: /* (sign_extend:M (ashiftrt:N (ashift <X> (const_int I)) (const_int I))) is (sign_extend:M (subreg:O <X>)) if there is mode with GET_MODE_BITSIZE (N) - I bits. (sign_extend:M (lshiftrt:N (ashift <X> (const_int I)) (const_int I))) is similarly (zero_extend:M (subreg:O <X>)). */ if ((GET_CODE (op) == ASHIFTRT || GET_CODE (op) == LSHIFTRT) && GET_CODE (XEXP (op, 0)) == ASHIFT && CONST_INT_P (XEXP (op, 1)) && XEXP (XEXP (op, 0), 1) == XEXP (op, 1) && GET_MODE_BITSIZE (GET_MODE (op)) > INTVAL (XEXP (op, 1))) { enum machine_mode tmode = mode_for_size (GET_MODE_BITSIZE (GET_MODE (op)) - INTVAL (XEXP (op, 1)), MODE_INT, 1); gcc_assert (GET_MODE_BITSIZE (mode) > GET_MODE_BITSIZE (GET_MODE (op))); if (tmode != BLKmode) { rtx inner = rtl_hooks.gen_lowpart_no_emit (tmode, XEXP (XEXP (op, 0), 0)); if (!inner) return NULL_RTX; return simplify_gen_unary (GET_CODE (op) == ASHIFTRT ? SIGN_EXTEND : ZERO_EXTEND, mode, inner, tmode); } } So perhaps we want it on stable branches too, after some time in trunk. Paolo > Fixed by making the hook instead return NULL_RTX if it wasn't possible to > generate the lowpart, and adjusting all the hook uses to cope with that. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2012-12-17 Jakub Jelinek <ja...@redhat.com> > > PR debug/55717 > * rtlhooks-def.h (RTL_HOOKS_GEN_LOWPART_NO_EMIT): Define to > gen_lowpart_if_possible. > (gen_lowpart_no_emit_general): Remove prototype. > * rtlhooks.c (gen_lowpart_no_emit_general): Removed. > * simplify-rtx.c (simplify_unary_operation_1, > simplify_binary_operation_1): Continue simplifying if > rtl_hooks.gen_lowpart_no_emit returns NULL_RTX. > * dwarf2out.c (mem_loc_descriptor) <case TRUNCATE>: Handle > truncation like lowpart SUBREG. > > * testsuite/g++.dg/opt/pr55717.C: New test. > > --- gcc/rtlhooks-def.h.jj 2009-02-20 15:40:11.000000000 +0100 > +++ gcc/rtlhooks-def.h 2012-12-17 19:34:38.852895461 +0100 > @@ -23,7 +23,7 @@ along with GCC; see the file COPYING3. > #include "rtl.h" > > #define RTL_HOOKS_GEN_LOWPART gen_lowpart_general > -#define RTL_HOOKS_GEN_LOWPART_NO_EMIT gen_lowpart_no_emit_general > +#define RTL_HOOKS_GEN_LOWPART_NO_EMIT gen_lowpart_if_possible > #define RTL_HOOKS_REG_NONZERO_REG_BITS reg_nonzero_bits_general > #define RTL_HOOKS_REG_NUM_SIGN_BIT_COPIES reg_num_sign_bit_copies_general > #define RTL_HOOKS_REG_TRUNCATED_TO_MODE reg_truncated_to_mode_general > @@ -38,7 +38,6 @@ along with GCC; see the file COPYING3. > } > > extern rtx gen_lowpart_general (enum machine_mode, rtx); > -extern rtx gen_lowpart_no_emit_general (enum machine_mode, rtx); > extern rtx reg_nonzero_bits_general (const_rtx, enum machine_mode, const_rtx, > enum machine_mode, > unsigned HOST_WIDE_INT, > --- gcc/rtlhooks.c.jj 2011-07-07 13:24:05.000000000 +0200 > +++ gcc/rtlhooks.c 2012-12-17 19:34:08.634071165 +0100 > @@ -79,18 +79,6 @@ gen_lowpart_general (enum machine_mode m > } > } > > -/* Similar to gen_lowpart, but cannot emit any instruction via > - copy_to_reg or force_reg. Mainly used in simplify-rtx.c. */ > -rtx > -gen_lowpart_no_emit_general (enum machine_mode mode, rtx x) > -{ > - rtx result = gen_lowpart_if_possible (mode, x); > - if (result) > - return result; > - else > - return x; > -} > - > rtx > reg_num_sign_bit_copies_general (const_rtx x ATTRIBUTE_UNUSED, > enum machine_mode mode ATTRIBUTE_UNUSED, > --- gcc/simplify-rtx.c.jj 2012-11-28 23:57:47.000000000 +0100 > +++ gcc/simplify-rtx.c 2012-12-17 18:10:48.873398378 +0100 > @@ -873,7 +873,9 @@ simplify_unary_operation_1 (enum rtx_cod > simplify_gen_unary (NOT, inner_mode, const1_rtx, > inner_mode), > XEXP (SUBREG_REG (op), 1)); > - return rtl_hooks.gen_lowpart_no_emit (mode, x); > + temp = rtl_hooks.gen_lowpart_no_emit (mode, x); > + if (temp) > + return temp; > } > > /* Apply De Morgan's laws to reduce number of patterns for machines > @@ -1029,7 +1031,11 @@ simplify_unary_operation_1 (enum rtx_cod > if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT) > { > if (TRULY_NOOP_TRUNCATION_MODES_P (mode, GET_MODE (op))) > - return rtl_hooks.gen_lowpart_no_emit (mode, op); > + { > + temp = rtl_hooks.gen_lowpart_no_emit (mode, op); > + if (temp) > + return temp; > + } > /* We can't handle truncation to a partial integer mode here > because we don't know the real bitsize of the partial > integer mode. */ > @@ -1048,7 +1054,11 @@ simplify_unary_operation_1 (enum rtx_cod > if (GET_MODE_NUNITS (mode) == 1 > && (TRULY_NOOP_TRUNCATION_MODES_P (mode, GET_MODE (op)) > || truncated_to_mode (mode, op))) > - return rtl_hooks.gen_lowpart_no_emit (mode, op); > + { > + temp = rtl_hooks.gen_lowpart_no_emit (mode, op); > + if (temp) > + return temp; > + } > > /* A truncate of a comparison can be replaced with a subreg if > STORE_FLAG_VALUE permits. This is like the previous test, > @@ -1057,7 +1067,11 @@ simplify_unary_operation_1 (enum rtx_cod > if (HWI_COMPUTABLE_MODE_P (mode) > && COMPARISON_P (op) > && (STORE_FLAG_VALUE & ~GET_MODE_MASK (mode)) == 0) > - return rtl_hooks.gen_lowpart_no_emit (mode, op); > + { > + temp = rtl_hooks.gen_lowpart_no_emit (mode, op); > + if (temp) > + return temp; > + } > > /* A truncate of a memory is just loading the low part of the memory > if we are not changing the meaning of the address. */ > @@ -1065,7 +1079,11 @@ simplify_unary_operation_1 (enum rtx_cod > && !VECTOR_MODE_P (mode) > && !MEM_VOLATILE_P (op) > && !mode_dependent_address_p (XEXP (op, 0), MEM_ADDR_SPACE (op))) > - return rtl_hooks.gen_lowpart_no_emit (mode, op); > + { > + temp = rtl_hooks.gen_lowpart_no_emit (mode, op); > + if (temp) > + return temp; > + } > > break; > > @@ -1298,7 +1316,11 @@ simplify_unary_operation_1 (enum rtx_cod > && SUBREG_PROMOTED_VAR_P (op) > && ! SUBREG_PROMOTED_UNSIGNED_P (op) > && GET_MODE_SIZE (mode) <= GET_MODE_SIZE (GET_MODE (XEXP (op, 0)))) > - return rtl_hooks.gen_lowpart_no_emit (mode, op); > + { > + temp = rtl_hooks.gen_lowpart_no_emit (mode, op); > + if (temp) > + return temp; > + } > > /* (sign_extend:M (sign_extend:N <X>)) is (sign_extend:M <X>). > (sign_extend:M (zero_extend:N <X>)) is (zero_extend:M <X>). */ > @@ -1330,9 +1352,10 @@ simplify_unary_operation_1 (enum rtx_cod > { > rtx inner = > rtl_hooks.gen_lowpart_no_emit (tmode, XEXP (XEXP (op, 0), 0)); > - return simplify_gen_unary (GET_CODE (op) == ASHIFTRT > - ? SIGN_EXTEND : ZERO_EXTEND, > - mode, inner, tmode); > + if (inner) > + return simplify_gen_unary (GET_CODE (op) == ASHIFTRT > + ? SIGN_EXTEND : ZERO_EXTEND, > + mode, inner, tmode); > } > } > > @@ -1360,7 +1383,11 @@ simplify_unary_operation_1 (enum rtx_cod > && SUBREG_PROMOTED_VAR_P (op) > && SUBREG_PROMOTED_UNSIGNED_P (op) > 0 > && GET_MODE_SIZE (mode) <= GET_MODE_SIZE (GET_MODE (XEXP (op, 0)))) > - return rtl_hooks.gen_lowpart_no_emit (mode, op); > + { > + temp = rtl_hooks.gen_lowpart_no_emit (mode, op); > + if (temp) > + return temp; > + } > > /* Extending a widening multiplication should be canonicalized to > a wider widening multiplication. */ > @@ -1425,7 +1452,8 @@ simplify_unary_operation_1 (enum rtx_cod > { > rtx inner = > rtl_hooks.gen_lowpart_no_emit (tmode, XEXP (XEXP (op, 0), 0)); > - return simplify_gen_unary (ZERO_EXTEND, mode, inner, tmode); > + if (inner) > + return simplify_gen_unary (ZERO_EXTEND, mode, inner, tmode); > } > } > > @@ -3095,7 +3123,11 @@ simplify_binary_operation_1 (enum rtx_co > } > /* x/1 is x. */ > if (trueop1 == CONST1_RTX (mode)) > - return rtl_hooks.gen_lowpart_no_emit (mode, op0); > + { > + tem = rtl_hooks.gen_lowpart_no_emit (mode, op0); > + if (tem) > + return tem; > + } > /* Convert divide by power of two into shift. */ > if (CONST_INT_P (trueop1) > && (val = exact_log2 (UINTVAL (trueop1))) > 0) > @@ -3154,12 +3186,17 @@ simplify_binary_operation_1 (enum rtx_co > } > /* x/1 is x. */ > if (trueop1 == CONST1_RTX (mode)) > - return rtl_hooks.gen_lowpart_no_emit (mode, op0); > + { > + tem = rtl_hooks.gen_lowpart_no_emit (mode, op0); > + if (tem) > + return tem; > + } > /* x/-1 is -x. */ > if (trueop1 == constm1_rtx) > { > rtx x = rtl_hooks.gen_lowpart_no_emit (mode, op0); > - return simplify_gen_unary (NEG, mode, x, mode); > + if (x) > + return simplify_gen_unary (NEG, mode, x, mode); > } > } > break; > --- gcc/dwarf2out.c.jj 2012-12-11 09:25:18.000000000 +0100 > +++ gcc/dwarf2out.c 2012-12-17 18:19:31.706376594 +0100 > @@ -11840,6 +11840,7 @@ mem_loc_descriptor (rtx rtl, enum machin > dw_loc_descr_ref mem_loc_result = NULL; > enum dwarf_location_atom op; > dw_loc_descr_ref op0, op1; > + rtx inner = NULL_RTX; > > if (mode == VOIDmode) > mode = GET_MODE (rtl); > @@ -11869,35 +11870,39 @@ mem_loc_descriptor (rtx rtl, enum machin > contains the given subreg. */ > if (!subreg_lowpart_p (rtl)) > break; > + inner = SUBREG_REG (rtl); > + case TRUNCATE: > + if (inner == NULL_RTX) > + inner = XEXP (rtl, 0); > if (GET_MODE_CLASS (mode) == MODE_INT > - && GET_MODE_CLASS (GET_MODE (SUBREG_REG (rtl))) == MODE_INT > + && GET_MODE_CLASS (GET_MODE (inner)) == MODE_INT > && (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE > #ifdef POINTERS_EXTEND_UNSIGNED > || (mode == Pmode && mem_mode != VOIDmode) > #endif > ) > - && GET_MODE_SIZE (GET_MODE (SUBREG_REG (rtl))) <= DWARF2_ADDR_SIZE) > + && GET_MODE_SIZE (GET_MODE (inner)) <= DWARF2_ADDR_SIZE) > { > - mem_loc_result = mem_loc_descriptor (SUBREG_REG (rtl), > - GET_MODE (SUBREG_REG (rtl)), > + mem_loc_result = mem_loc_descriptor (inner, > + GET_MODE (inner), > mem_mode, initialized); > break; > } > if (dwarf_strict) > break; > - if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (SUBREG_REG (rtl)))) > + if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (inner))) > break; > - if (GET_MODE_SIZE (mode) != GET_MODE_SIZE (GET_MODE (SUBREG_REG (rtl))) > + if (GET_MODE_SIZE (mode) != GET_MODE_SIZE (GET_MODE (inner)) > && (GET_MODE_CLASS (mode) != MODE_INT > - || GET_MODE_CLASS (GET_MODE (SUBREG_REG (rtl))) != MODE_INT)) > + || GET_MODE_CLASS (GET_MODE (inner)) != MODE_INT)) > break; > else > { > dw_die_ref type_die; > dw_loc_descr_ref cvt; > > - mem_loc_result = mem_loc_descriptor (SUBREG_REG (rtl), > - GET_MODE (SUBREG_REG (rtl)), > + mem_loc_result = mem_loc_descriptor (inner, > + GET_MODE (inner), > mem_mode, initialized); > if (mem_loc_result == NULL) > break; > @@ -11909,7 +11914,7 @@ mem_loc_descriptor (rtx rtl, enum machin > break; > } > if (GET_MODE_SIZE (mode) > - != GET_MODE_SIZE (GET_MODE (SUBREG_REG (rtl)))) > + != GET_MODE_SIZE (GET_MODE (inner))) > cvt = new_loc_descr (DW_OP_GNU_convert, 0, 0); > else > cvt = new_loc_descr (DW_OP_GNU_reinterpret, 0, 0); > @@ -12666,7 +12671,6 @@ mem_loc_descriptor (rtx rtl, enum machin > break; > > case COMPARE: > - case TRUNCATE: > /* In theory, we could implement the above. */ > /* DWARF cannot represent the unsigned compare operations > natively. */ > --- gcc/testsuite/g++.dg/opt/pr55717.C.jj 2012-12-17 19:27:04.160509897 > +0100 > +++ gcc/testsuite/g++.dg/opt/pr55717.C 2012-12-17 19:26:36.000000000 > +0100 > @@ -0,0 +1,107 @@ > +// PR debug/55717 > +// { dg-do compile } > +// { dg-options "-O -g" } > + > +struct DebugOnly {}; > +template <class T> > +struct StripConst { typedef T result; }; > +class TempAllocPolicy {}; > +template <class T> > +class HashTableEntry > +{ > + unsigned keyHash; > + template <class, class, class> > + friend class HashTable; > + T t; > + void setLive (unsigned hn) { keyHash = hn; } > +}; > +template <class T, class HashPolicy, class> > +struct HashTable > +{ > + typedef typename HashPolicy::KeyType Key; > + typedef typename HashPolicy::Lookup Lookup; > + typedef HashTableEntry <T> Entry; > + struct Range > + { > + Range () {} > + Entry *cur, end; > + bool empty () { return false; } > + T front () { return T (); } > + }; > + struct Enum : public Range > + { > + HashTable table; > + bool removed; > + template <class Map> > + Enum (Map map) : Range (map.all ()), table (map.impl), removed () {} > + void rekeyFront (Lookup l, Key) > + { > + T t = this->cur->t; > + table.putNewInfallible (l, t); > + } > + void rekeyFront (Key k) > + { > + rekeyFront (k, k); > + } > + }; > + unsigned entryCount; > + unsigned sCollisionBit; > + unsigned prepareHash (Lookup l) > + { > + unsigned keyHash (HashPolicy::hash (l)); > + return keyHash & sCollisionBit; > + } > + static Entry *entryp; > + Entry *findFreeEntry (unsigned) { return entryp; } > + void putNewInfallible (Lookup l, T) > + { > + unsigned keyHash = prepareHash (l); > + Entry *entry = findFreeEntry (keyHash); > + entry->setLive (keyHash); > + entryCount++; > + } > +}; > +template <class Key> > +struct HashMapEntry { Key key; }; > +template <class Key, class Value, class HashPolicy = DebugOnly, class > AllocPolicy = TempAllocPolicy> > +struct HashMap > +{ > + typedef HashMapEntry <Key> Entry; > + struct MapHashPolicy : HashPolicy > + { > + typedef Key KeyType; > + }; > + typedef HashTable <Entry, MapHashPolicy, AllocPolicy> Impl; > + Impl impl; > + typedef typename Impl::Range Range; > + Range all () { return Range (); } > + typedef typename Impl::Enum Enum; > +}; > +class FreeOp; > +struct AllocationSiteKey; > +typedef HashMap <AllocationSiteKey, DebugOnly, AllocationSiteKey, > TempAllocPolicy> AllocationSiteTable; > +struct TypeCompartment > +{ > + AllocationSiteTable *allocationSiteTable; > + void sweep (FreeOp *); > +}; > +struct JSScript { unsigned *code; }; > +bool IsScriptMarked (JSScript **); > +struct AllocationSiteKey > +{ > + JSScript *script; > + unsigned offset : 24; > + int kind; > + typedef AllocationSiteKey Lookup; > + static unsigned hash (AllocationSiteKey key) { return (long > (key.script->code + key.offset)) ^ key.kind; } > +}; > +void > +TypeCompartment::sweep (FreeOp *) > +{ > + for (AllocationSiteTable::Enum e (*allocationSiteTable); !e.empty ();) > + { > + AllocationSiteKey key = e.front ().key; > + IsScriptMarked (&key.script); > + e.rekeyFront (key); > + } > +} > > Jakub >