On Thu, Jan 31, 2019 at 3:45 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Thu, Jan 31, 2019 at 09:30:13AM -0500, Aldy Hernandez wrote:
> > > > (LOOPS_HAVE_PREHEADERS). This avoids unnecessary changes to the CFG.
> > > > But really, I'm just guessing here. LOOPS_NORMAL would also work,
> > > > albeit creating forwarder blocks.
> > > >
> > > > Tested on x86-64 Linux.
> > > >
> > > > What do you think?
> > >
> > > As far as I understand fold_return_value is true/false independent
> > > of -W* flags (but dependent on -fprintf-return-value or so). This means
> > > that your patch avoids the risk of CFG changes dependent on
> > > -W* flags. This means you could safely use LOOPS_NORMAL
> >
> > But isn't my code inside of ::execute, which is still gated by
> > fold_return_value AND all the -W* flags:?
> >
> > bool
> > pass_sprintf_length::gate (function *)
> > {
> > return ((warn_format_overflow > 0
> > || warn_format_trunc > 0
> > || flag_printf_return_value)
> > && (optimize > 0) == fold_return_value);
> > }
> >
> > Thus, I think we need to move the loop initialization somewhere else ??.
>
> Then perhaps turn the gate into just return (optimize > 0) ==
> fold_return_value;
> and in execute do what you're adding and guard the rest with the above
> condition? As -fprintf-return-value is on by default for C-family, it
> shouldn't
> change much.
> + adjust comment on the gate of course.
Don't you alreday have
@@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun)
init_target_to_host_charmap ();
calculate_dominance_info (CDI_DOMINATORS);
+ bool optimizing_late = optimize > 0 && fold_return_value;
+ if (optimizing_late)
+ {
+ /* ?? We should avoid changing the CFG as much as possible.
...
+ loop_optimizer_init (LOOPS_HAVE_PREHEADERS);
+ scev_initialize ();
+ }
so loops are only initialized if fold_return_value is true? Ah - but that's
the pass parameter from params.def rather than the flag to enable
the folding... So indeed, change it to include && flag_printf_return_value
Richard.
> Jakub
