This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix bootstrap miscompare by STV (PR91580)


On Thu, 29 Aug 2019, Uros Bizjak wrote:

> On Thu, Aug 29, 2019 at 12:04 PM Richard Biener <rguenther@suse.de> wrote:
> >
> >
> > The following fixes the bootstrap-debug miscompare caused by STV
> > where we ended up with chain-to-scalar copies just because of
> > debug uses.  Instead we have to avoid that, eventually substituting
> > into debug uses or resetting debug stmts when there are reaching
> > defs from both inside and outside of the chain (since we rename
> > all in-chain defs).
> >
> > Bootstrapped on i686-linux-gnu (with a setup previously
> > reproducing the miscompare).  Bootstrapped on x86_64-unknown-linux-gnu,
> > testing in progress.
> >
> > OK for trunk?
> >
> > Thanks,
> > Richard.
> >
> > 2019-08-29  Richard Biener  <rguenther@suse.de>
> >
> >         PR bootstrap/91580
> >         * config/i386/i386-features.c (general_scalar_chain::convert_insn):
> >         Do not emit scalar copies for debug-insns, instead replace
> >         their uses with the reg copy used in the chain or reset them
> >         if there is a reaching definition outside of the chain as well.
> 
> OK.

r275030.

> Let's fix the breakage, and maybe later we could look into merging
> with TImode debug fixups (which looks similar to the functionality in
> this patch).

I don't think that's easily possible since the TImode chains still
work in the way of having all defs of a pseudo in a chain, so
code generation and replacement is different.  Rather TImode
chains could be handled by the generic machinery by only making
loads, stores and reg-reg copies candidates.

Richard.

> Thanks,
> Uros.
> 
> > Index: gcc/config/i386/i386-features.c
> > ===================================================================
> > --- gcc/config/i386/i386-features.c     (revision 274991)
> > +++ gcc/config/i386/i386-features.c     (working copy)
> > @@ -880,18 +880,52 @@ general_scalar_chain::convert_op (rtx *o
> >  void
> >  general_scalar_chain::convert_insn (rtx_insn *insn)
> >  {
> > -  /* Generate copies for out-of-chain uses of defs.  */
> > +  /* Generate copies for out-of-chain uses of defs and adjust debug uses.  */
> >    for (df_ref ref = DF_INSN_DEFS (insn); ref; ref = DF_REF_NEXT_LOC (ref))
> >      if (bitmap_bit_p (defs_conv, DF_REF_REGNO (ref)))
> >        {
> >         df_link *use;
> >         for (use = DF_REF_CHAIN (ref); use; use = use->next)
> > -         if (DF_REF_REG_MEM_P (use->ref)
> > -             || !bitmap_bit_p (insns, DF_REF_INSN_UID (use->ref)))
> > +         if (NONDEBUG_INSN_P (DF_REF_INSN (use->ref))
> > +             && (DF_REF_REG_MEM_P (use->ref)
> > +                 || !bitmap_bit_p (insns, DF_REF_INSN_UID (use->ref))))
> >             break;
> >         if (use)
> >           convert_reg (insn, DF_REF_REG (ref),
> >                        *defs_map.get (regno_reg_rtx [DF_REF_REGNO (ref)]));
> > +       else
> > +         {
> > +           /* If we generated a scalar copy we can leave debug-insns
> > +              as-is, if not, we have to adjust them.  */
> > +           auto_vec<rtx_insn *, 5> to_reset_debug_insns;
> > +           for (use = DF_REF_CHAIN (ref); use; use = use->next)
> > +             if (DEBUG_INSN_P (DF_REF_INSN (use->ref)))
> > +               {
> > +                 rtx_insn *debug_insn = DF_REF_INSN (use->ref);
> > +                 /* If there's a reaching definition outside of the
> > +                    chain we have to reset.  */
> > +                 df_link *def;
> > +                 for (def = DF_REF_CHAIN (use->ref); def; def = def->next)
> > +                   if (!bitmap_bit_p (insns, DF_REF_INSN_UID (def->ref)))
> > +                     break;
> > +                 if (def)
> > +                   to_reset_debug_insns.safe_push (debug_insn);
> > +                 else
> > +                   {
> > +                     *DF_REF_REAL_LOC (use->ref)
> > +                       = *defs_map.get (regno_reg_rtx [DF_REF_REGNO (ref)]);
> > +                     df_insn_rescan (debug_insn);
> > +                   }
> > +               }
> > +           /* Have to do the reset outside of the DF_CHAIN walk to not
> > +              disrupt it.  */
> > +           while (!to_reset_debug_insns.is_empty ())
> > +             {
> > +               rtx_insn *debug_insn = to_reset_debug_insns.pop ();
> > +               INSN_VAR_LOCATION_LOC (debug_insn) = gen_rtx_UNKNOWN_VAR_LOC ();
> > +               df_insn_rescan_debug_internal (debug_insn);
> > +             }
> > +         }
> >        }
> >
> >    /* Replace uses in this insn with the defs we use in the chain.  */
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG München)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]