On Thu, 29 Aug 2019, Uros Bizjak wrote:
> On Thu, Aug 29, 2019 at 12:04 PM Richard Biener <rguent...@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 <rguent...@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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG München)
