| Message ID | 87vb7qsssx.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 12/22/2015 03:04 AM, Nick Clifton wrote: > Hi Guys, > > The patch below fixes PR 68770 - a warning from valgrind about an > uninitialised value being used in the default_secondary_reload. The > problem turns out to the in copy_costs which creates its own secondary > reload information structure, but it does not initialise all of the > fields. One field in particular - t_icode - is examined by > default_secondary_reload, and it was this that was triggering the > valgrind warning. > > Tested with no regressions on a x86_64-pc-linux-gnu and a > powerpc64-le-linux-gnu toolchain. > > OK to apply ? > > Cheers > Nick > > gcc/ChangeLog > 2015-12-22 Nick Clifton <nickc@redhat.com> > > PR target/68770 > * ira-costs.c (copy_cost): Initialise the t_code field of the sri > structure. Can you please reduce the testcase from the BZ so that it can be included in the testsuite? I realize it might take a long time with multidelta because of the need to run the compiler with valgrind. But once you've got the right script, you ought to be able to just let it run overnight or whatever. To speed things up, you might consider first trying to compile the case without valgrind and if that fails, then that particular reduction is "not interesting". That should dramatically cut down on the number of times you have to run the compiler under valgrind control. With a testcase, this is fine. jeff
On 2016.01.01 at 22:33 -0700, Jeff Law wrote: > On 12/22/2015 03:04 AM, Nick Clifton wrote: > >Hi Guys, > > > > The patch below fixes PR 68770 - a warning from valgrind about an > > uninitialised value being used in the default_secondary_reload. The > > problem turns out to the in copy_costs which creates its own secondary > > reload information structure, but it does not initialise all of the > > fields. One field in particular - t_icode - is examined by > > default_secondary_reload, and it was this that was triggering the > > valgrind warning. > > > > Tested with no regressions on a x86_64-pc-linux-gnu and a > > powerpc64-le-linux-gnu toolchain. > > > > OK to apply ? > > > >Cheers > > Nick > > > >gcc/ChangeLog > >2015-12-22 Nick Clifton <nickc@redhat.com> > > > > PR target/68770 > > * ira-costs.c (copy_cost): Initialise the t_code field of the sri > > structure. > Can you please reduce the testcase from the BZ so that it can be included in > the testsuite? I realize it might take a long time with multidelta because > of the need to run the compiler with valgrind. But once you've got the > right script, you ought to be able to just let it run overnight or whatever. > > To speed things up, you might consider first trying to compile the case > without valgrind and if that fails, then that particular reduction is "not > interesting". That should dramatically cut down on the number of times you > have to run the compiler under valgrind control. > > With a testcase, this is fine. trippels@gcc2-power8 ~ % cat parallel_settings.ii struct A { unsigned long a; float f; A() : f(0.01f) {} }; A s; trippels@gcc2-power8 ~ % valgrind -q --trace-children=yes g++ -O2 -c parallel_settings.ii ==149056== Conditional jump or move depends on uninitialised value(s) ==149056== at 0x10A78484: default_secondary_reload(bool, rtx_def*, int, machine_mode, secondary_reload_info*) (targhooks.c:940) ==149056== by 0x10E50D03: rs6000_secondary_reload(bool, rtx_def*, int, machine_mode, secondary_reload_info*) (rs6000.c:18414) ==149056== by 0x1085F137: copy_cost(rtx_def*, machine_mode, int, bool, secondary_reload_info*) [clone .part.7] (ira-costs.c:445) ==149056== by 0x1085F1A7: copy_cost (ira-costs.c:434) ==149056== by 0x1085F1A7: copy_cost(rtx_def*, machine_mode, int, bool, secondary_reload_info*) [clone .part.7] (ira-costs.c:452) ==149056== by 0x1085F6E3: copy_cost (ira-costs.c:434) ==149056== by 0x1085F6E3: record_reg_classes(int, int, rtx_def**, machine_mode*, char const**, rtx_insn*, reg_class*) [clone .constprop.43] (ira-costs.c:984) ==149056== by 0x10861A7F: record_operand_costs(rtx_insn*, reg_class*) (ira-costs.c:1325) ==149056== by 0x10861FCB: scan_one_insn (ira-costs.c:1462) ==149056== by 0x10861FCB: process_bb_for_costs(basic_block_def*) (ira-costs.c:1583) ==149056== by 0x10857D2F: ira_traverse_loop_tree(bool, ira_loop_tree_node*, void (*)(ira_loop_tree_node*), void (*)(ira_loop_tree_node*)) (ira-build.c:1798) ==149056== by 0x108649FF: find_costs_and_classes(_IO_FILE*) (ira-costs.c:1680) ==149056== by 0x108651B3: ira_costs() (ira-costs.c:2213) ==149056== by 0x1085C09B: ira_build() (ira-build.c:3419) ==149056== by 0x10851D3B: ira (ira.c:5221) ==149056== by 0x10851D3B: (anonymous namespace)::pass_ira::execute(function*) (ira.c:5513) ==149056==
Index: ira-costs.c =================================================================== --- ira-costs.c (revision 231898) +++ ira-costs.c (working copy) @@ -442,6 +442,9 @@ copy it. */ sri.prev_sri = prev_sri; sri.extra_cost = 0; + /* PR 68770: Secondary reload might examine the t_icode field. */ + sri.t_icode = CODE_FOR_nothing; + secondary_class = targetm.secondary_reload (to_p, x, rclass, mode, &sri); if (secondary_class != NO_REGS)