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
> 

Reply via email to