[PATCH] Don't return original value from gen_lowpart_no_emit rtl hook (PR debug/55717)
Paolo Bonzini
bonzini@gnu.org
Tue Dec 18 08:25:00 GMT 2012
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 <jakub@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
>
More information about the Gcc-patches
mailing list