* [PATCH] Avoid uninit warnings with PR79345 fix
@ 2017-02-21 13:54 Richard Biener
2017-02-21 14:26 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2017-02-21 13:54 UTC (permalink / raw)
To: gcc-patches
The following fixes a bit-load.c bug as well as avoids the warnings
for two other cases.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Ok?
Thanks,
Richard.
2017-02-21 Richard Biener <rguenther@suse.de>
* bt-load.c (compute_defs_uses_and_gen): Clear call_saved.
* fixed-value.c (fixed_from_string): Use wi::ulow and wi::uhigh.
(fixed_convert_from_real): Likewise.
* ipa-cp.c (ipcp_store_vr_results): Do not uselessly initialize
VR_VARYING min/max.
Index: gcc/bt-load.c
===================================================================
--- gcc/bt-load.c (revision 245620)
+++ gcc/bt-load.c (working copy)
@@ -543,6 +543,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
int i;
/* Check for sibcall. */
+ CLEAR_HARD_REG_SET (call_saved);
if (GET_CODE (pat) == PARALLEL)
for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
Index: gcc/fixed-value.c
===================================================================
--- gcc/fixed-value.c (revision 245620)
+++ gcc/fixed-value.c (working copy)
@@ -130,8 +130,8 @@ fixed_from_string (FIXED_VALUE_TYPE *f,
real_arithmetic (&fixed_value, MULT_EXPR, &real_value, &base_value);
wide_int w = real_to_integer (&fixed_value, &fail,
GET_MODE_PRECISION (mode));
- f->data.low = w.elt (0);
- f->data.high = w.elt (1);
+ f->data.low = w.ulow ();
+ f->data.high = w.uhigh ();
if (temp == FIXED_MAX_EPS && ALL_FRACT_MODE_P (f->mode))
{
@@ -1049,8 +1049,8 @@ fixed_convert_from_real (FIXED_VALUE_TYP
wide_int w = real_to_integer (&fixed_value, &fail,
GET_MODE_PRECISION (mode));
- f->data.low = w.elt (0);
- f->data.high = w.elt (1);
+ f->data.low = w.ulow ();
+ f->data.high = w.uhigh ();
temp = check_real_for_fixed_mode (&real_value, mode);
if (temp == FIXED_UNDERFLOW) /* Minimum. */
{
Index: gcc/ipa-cp.c
===================================================================
--- gcc/ipa-cp.c (revision 245620)
+++ gcc/ipa-cp.c (working copy)
@@ -4959,7 +4959,6 @@ ipcp_store_vr_results (void)
{
vr.known = false;
vr.type = VR_VARYING;
- vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
}
ts->m_vr->quick_push (vr);
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Avoid uninit warnings with PR79345 fix
2017-02-21 13:54 [PATCH] Avoid uninit warnings with PR79345 fix Richard Biener
@ 2017-02-21 14:26 ` Jakub Jelinek
2017-02-21 14:47 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2017-02-21 14:26 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Tue, Feb 21, 2017 at 02:53:32PM +0100, Richard Biener wrote:
>
> The following fixes a bit-load.c bug as well as avoids the warnings
> for two other cases.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Ok?
>
> Thanks,
> Richard.
>
> 2017-02-21 Richard Biener <rguenther@suse.de>
>
> * bt-load.c (compute_defs_uses_and_gen): Clear call_saved.
> * fixed-value.c (fixed_from_string): Use wi::ulow and wi::uhigh.
> (fixed_convert_from_real): Likewise.
> * ipa-cp.c (ipcp_store_vr_results): Do not uselessly initialize
> VR_VARYING min/max.
>
> Index: gcc/bt-load.c
> ===================================================================
> --- gcc/bt-load.c (revision 245620)
> +++ gcc/bt-load.c (working copy)
> @@ -543,6 +543,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
> int i;
>
> /* Check for sibcall. */
> + CLEAR_HARD_REG_SET (call_saved);
> if (GET_CODE (pat) == PARALLEL)
> for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
> if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
Why do we warn here?
HARD_REG_SET *clobbered = &call_used_reg_set;
HARD_REG_SET call_saved;
rtx pat = PATTERN (insn);
int i;
/* Check for sibcall. */
if (GET_CODE (pat) == PARALLEL)
for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
{
COMPL_HARD_REG_SET (call_saved,
call_used_reg_set);
clobbered = &call_saved;
}
for (regno = first_btr; regno <= last_btr; regno++)
if (TEST_HARD_REG_BIT (*clobbered, regno))
note_btr_set (regno_reg_rtx[regno], NULL_RTX, &info);
COMPL_HARD_REG_SET should always overwrite all of call_saved (it is
call_saved = ~call_used_reg_set).
> --- gcc/fixed-value.c (revision 245620)
> +++ gcc/fixed-value.c (working copy)
> @@ -130,8 +130,8 @@ fixed_from_string (FIXED_VALUE_TYPE *f,
> real_arithmetic (&fixed_value, MULT_EXPR, &real_value, &base_value);
> wide_int w = real_to_integer (&fixed_value, &fail,
> GET_MODE_PRECISION (mode));
> - f->data.low = w.elt (0);
> - f->data.high = w.elt (1);
> + f->data.low = w.ulow ();
> + f->data.high = w.uhigh ();
Is this for the case when the wide_int has only a single uhwi (or more than
two)?
> --- gcc/ipa-cp.c (revision 245620)
> +++ gcc/ipa-cp.c (working copy)
> @@ -4959,7 +4959,6 @@ ipcp_store_vr_results (void)
> {
> vr.known = false;
> vr.type = VR_VARYING;
> - vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
> }
> ts->m_vr->quick_push (vr);
> }
It is strange to see removing initialization of something as a work-around
to uninitialized warning. Is that because of the vr.min = vr.max assignment
and that wi::zero doesn't initialize everything?
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Avoid uninit warnings with PR79345 fix
2017-02-21 14:26 ` Jakub Jelinek
@ 2017-02-21 14:47 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2017-02-21 14:47 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Tue, 21 Feb 2017, Jakub Jelinek wrote:
> On Tue, Feb 21, 2017 at 02:53:32PM +0100, Richard Biener wrote:
> >
> > The following fixes a bit-load.c bug as well as avoids the warnings
> > for two other cases.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Ok?
> >
> > Thanks,
> > Richard.
> >
> > 2017-02-21 Richard Biener <rguenther@suse.de>
> >
> > * bt-load.c (compute_defs_uses_and_gen): Clear call_saved.
> > * fixed-value.c (fixed_from_string): Use wi::ulow and wi::uhigh.
> > (fixed_convert_from_real): Likewise.
> > * ipa-cp.c (ipcp_store_vr_results): Do not uselessly initialize
> > VR_VARYING min/max.
> >
> > Index: gcc/bt-load.c
> > ===================================================================
> > --- gcc/bt-load.c (revision 245620)
> > +++ gcc/bt-load.c (working copy)
> > @@ -543,6 +543,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
> > int i;
> >
> > /* Check for sibcall. */
> > + CLEAR_HARD_REG_SET (call_saved);
> > if (GET_CODE (pat) == PARALLEL)
> > for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
> > if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
>
> Why do we warn here?
> HARD_REG_SET *clobbered = &call_used_reg_set;
> HARD_REG_SET call_saved;
> rtx pat = PATTERN (insn);
> int i;
>
> /* Check for sibcall. */
> if (GET_CODE (pat) == PARALLEL)
> for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
> if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
> {
> COMPL_HARD_REG_SET (call_saved,
> call_used_reg_set);
> clobbered = &call_saved;
> }
>
> for (regno = first_btr; regno <= last_btr; regno++)
> if (TEST_HARD_REG_BIT (*clobbered, regno))
> note_btr_set (regno_reg_rtx[regno], NULL_RTX, &info);
> COMPL_HARD_REG_SET should always overwrite all of call_saved (it is
> call_saved = ~call_used_reg_set).
Not sure why we warn, didn't fully investigate.
> > --- gcc/fixed-value.c (revision 245620)
> > +++ gcc/fixed-value.c (working copy)
> > @@ -130,8 +130,8 @@ fixed_from_string (FIXED_VALUE_TYPE *f,
> > real_arithmetic (&fixed_value, MULT_EXPR, &real_value, &base_value);
> > wide_int w = real_to_integer (&fixed_value, &fail,
> > GET_MODE_PRECISION (mode));
> > - f->data.low = w.elt (0);
> > - f->data.high = w.elt (1);
> > + f->data.low = w.ulow ();
> > + f->data.high = w.uhigh ();
>
> Is this for the case when the wide_int has only a single uhwi (or more than
> two)?
This is for the case where elt (0) does
template <typename storage>
inline HOST_WIDE_INT
generic_wide_int <storage>::elt (unsigned int i) const
{
if (i >= this->get_len ())
return sign_mask ();
turning that into 0 == this->get_len () and sign_mask has
inline HOST_WIDE_INT
generic_wide_int <storage>::sign_mask () const
{
unsigned int len = this->get_len ();
unsigned HOST_WIDE_INT high = this->get_val ()[len - 1];
so we propagate len == 0 here and access this out-of-bound (and nothing
knows len is never zero).
> > --- gcc/ipa-cp.c (revision 245620)
> > +++ gcc/ipa-cp.c (working copy)
> > @@ -4959,7 +4959,6 @@ ipcp_store_vr_results (void)
> > {
> > vr.known = false;
> > vr.type = VR_VARYING;
> > - vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
> > }
> > ts->m_vr->quick_push (vr);
> > }
>
> It is strange to see removing initialization of something as a work-around
> to uninitialized warning. Is that because of the vr.min = vr.max assignment
> and that wi::zero doesn't initialize everything?
didn't fully track this down but it looks like the assignment from
wi::hwi_with_prec somehow accesses uninitialized memory.
I can only spend more time with this later this week (or next week).
Changing the warning patch to add a %K helps (you get at least an idea
of the inline stack):
+ /* We didn't find any may-defs so on all paths either
+ reached function entry or a killing clobber. */
if (always_executed)
- warn_uninit (OPT_Wuninitialized, use, gimple_assign_rhs1
(stmt),
- base, "%qE is used uninitialized in this
function",
- stmt, UNKNOWN_LOCATION);
+ warning_at (gimple_location (stmt), OPT_Wuninitialized,
+ "%K%qE is used uninitialized in this
function",
+ rhs, rhs);
else if (warn_possibly_uninitialized)
- warn_uninit (OPT_Wmaybe_uninitialized, use,
- gimple_assign_rhs1 (stmt), base,
- "%qE may be used uninitialized in this
function",
- stmt, UNKNOWN_LOCATION);
+ warning_at (gimple_location (stmt),
OPT_Wmaybe_uninitialized,
+ "%K%qE may be used uninitialized in this
function",
+ rhs, rhs);
Richard.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-21 14:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-21 13:54 [PATCH] Avoid uninit warnings with PR79345 fix Richard Biener
2017-02-21 14:26 ` Jakub Jelinek
2017-02-21 14:47 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).