| Message ID | eb22aac6-891c-7c92-896d-831ccdc12047@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [GCC/testsuite] Fix dump-noaddr dumpbase | expand |
On Tue, Dec 5, 2017 at 9:50 AM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > Hi, > > dump-noaddr test FAILS when $tmpdir is not the same as the directory > where runtest is called from. Note that this does not happen when > running make check because tmpdir is set to srcdir. > > In that case, file mkdir will create the directory in the current > directory while GCC is invoked from tmpdir and hence -dumpbase look > for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to > be relative to tmpdir which will work in all case. > > ChangeLog entry is as follows: > > *** gcc/testsuite/ChangeLog *** > > 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base > relative to tmpdir. > > Testing: Successfully ran unsorted.exp via make check and out of tree > testing using runtest from <path>/test with tmpdir set in > <path>/test/site.exp to <path>. > > Is this ok for stage3? https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html I don't remember where this discussion went last time. Maybe this time there will be a resolution :). Thanks, Andrew > > Best regards, > > Thomas
On 05/12/17 17:54, Andrew Pinski wrote: > On Tue, Dec 5, 2017 at 9:50 AM, Thomas Preudhomme > <thomas.preudhomme@foss.arm.com> wrote: >> Hi, >> >> dump-noaddr test FAILS when $tmpdir is not the same as the directory >> where runtest is called from. Note that this does not happen when >> running make check because tmpdir is set to srcdir. >> >> In that case, file mkdir will create the directory in the current >> directory while GCC is invoked from tmpdir and hence -dumpbase look >> for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to >> be relative to tmpdir which will work in all case. >> >> ChangeLog entry is as follows: >> >> *** gcc/testsuite/ChangeLog *** >> >> 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base >> relative to tmpdir. >> >> Testing: Successfully ran unsorted.exp via make check and out of tree >> testing using runtest from <path>/test with tmpdir set in >> <path>/test/site.exp to <path>. >> >> Is this ok for stage3? > > https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html > I don't remember where this discussion went last time. > Maybe this time there will be a resolution :). FWIW, I agree with Matt, creating the dump in tmpdir makes more sense. I think his patch can be simplified though because the compiler seems to be invoked from tmpdir so it can at least be omitted from the -dumpbase. Best regards, Thomas
On Dec 5, 2017, at 9:54 AM, Andrew Pinski <pinskia@gmail.com> wrote: > > On Tue, Dec 5, 2017 at 9:50 AM, Thomas Preudhomme > <thomas.preudhomme@foss.arm.com> wrote: >> Hi, >> >> dump-noaddr test FAILS when $tmpdir is not the same as the directory >> where runtest is called from. Note that this does not happen when >> running make check because tmpdir is set to srcdir. >> >> In that case, file mkdir will create the directory in the current >> directory while GCC is invoked from tmpdir and hence -dumpbase look >> for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to >> be relative to tmpdir which will work in all case. >> >> ChangeLog entry is as follows: >> >> *** gcc/testsuite/ChangeLog *** >> >> 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base >> relative to tmpdir. >> >> Testing: Successfully ran unsorted.exp via make check and out of tree >> testing using runtest from <path>/test with tmpdir set in >> <path>/test/site.exp to <path>. >> >> Is this ok for stage3? > > https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html > I don't remember where this discussion went last time. I approved his patch last time in: https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html I've not seen anyone argue against it. > Maybe this time there will be a resolution :). A resolution would be for someone to check in the approved patch, or ask for it to be installed. :-o I've checked it in. It might not work for canadian crosses, but we can punt that to the next poor soul. Give it a try and let us know if that cures the problem. Feel free to just fix it, if you notice anything wrong. I'm thinking it will cure all the problems people have seen.
On Dec 5, 2017, at 11:11 AM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > > On 05/12/17 17:54, Andrew Pinski wrote: >> On Tue, Dec 5, 2017 at 9:50 AM, Thomas Preudhomme >> <thomas.preudhomme@foss.arm.com> wrote: >>> Hi, >>> >>> dump-noaddr test FAILS when $tmpdir is not the same as the directory >>> where runtest is called from. Note that this does not happen when >>> running make check because tmpdir is set to srcdir. >>> >>> In that case, file mkdir will create the directory in the current >>> directory while GCC is invoked from tmpdir and hence -dumpbase look >>> for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to >>> be relative to tmpdir which will work in all case. >>> >>> ChangeLog entry is as follows: >>> >>> *** gcc/testsuite/ChangeLog *** >>> >>> 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> >>> >>> * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base >>> relative to tmpdir. >>> >>> Testing: Successfully ran unsorted.exp via make check and out of tree >>> testing using runtest from <path>/test with tmpdir set in >>> <path>/test/site.exp to <path>. >>> >>> Is this ok for stage3? >> https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html >> I don't remember where this discussion went last time. >> Maybe this time there will be a resolution :). > > FWIW, I agree with Matt, creating the dump in tmpdir makes more sense. I think his patch can be simplified though because the compiler seems to be invoked from tmpdir so it can at least be omitted from the -dumpbase. Sounds reasonable. I've added that on top of his patch and checked that in. Let us know if it works or not.
Hi Mike, Thanks, I've tested after the two commits and it works both in tree and out of tree. It'll simplify comparing in tree results Vs out of tree for us, thanks a lot! Would you consider a backport to stable branches if nobody complains after a week? Best regards, Thomas On 05/12/17 19:27, Mike Stump wrote: > On Dec 5, 2017, at 11:11 AM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: >> >> On 05/12/17 17:54, Andrew Pinski wrote: >>> On Tue, Dec 5, 2017 at 9:50 AM, Thomas Preudhomme >>> <thomas.preudhomme@foss.arm.com> wrote: >>>> Hi, >>>> >>>> dump-noaddr test FAILS when $tmpdir is not the same as the directory >>>> where runtest is called from. Note that this does not happen when >>>> running make check because tmpdir is set to srcdir. >>>> >>>> In that case, file mkdir will create the directory in the current >>>> directory while GCC is invoked from tmpdir and hence -dumpbase look >>>> for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to >>>> be relative to tmpdir which will work in all case. >>>> >>>> ChangeLog entry is as follows: >>>> >>>> *** gcc/testsuite/ChangeLog *** >>>> >>>> 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> >>>> >>>> * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base >>>> relative to tmpdir. >>>> >>>> Testing: Successfully ran unsorted.exp via make check and out of tree >>>> testing using runtest from <path>/test with tmpdir set in >>>> <path>/test/site.exp to <path>. >>>> >>>> Is this ok for stage3? >>> https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html >>> I don't remember where this discussion went last time. >>> Maybe this time there will be a resolution :). >> >> FWIW, I agree with Matt, creating the dump in tmpdir makes more sense. I think his patch can be simplified though because the compiler seems to be invoked from tmpdir so it can at least be omitted from the -dumpbase. > > Sounds reasonable. I've added that on top of his patch and checked that in. Let us know if it works or not. >
On Dec 5, 2017, at 12:56 PM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > > Thanks, I've tested after the two commits and it works both in tree and out of tree. It'll simplify comparing in tree results Vs out of tree for us, thanks a lot! > > Would you consider a backport to stable branches if nobody complains after a week? Yeah, back port is Ok.
Finally committed to gcc-7-branch, sorry for doing this so late. I've merged the two commits into one. Patch attached for reference. Best regards, Thomas On 05/12/17 21:26, Mike Stump wrote: > On Dec 5, 2017, at 12:56 PM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: >> >> Thanks, I've tested after the two commits and it works both in tree and out of tree. It'll simplify comparing in tree results Vs out of tree for us, thanks a lot! >> >> Would you consider a backport to stable branches if nobody complains after a week? > > Yeah, back port is Ok. > diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index b211dec4ffb20359f50bbc695481977282eb0525..b78c5f59bfc1121cf61071e41bd11551a9ab7122 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2017-02-27 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-12-05 Matthew Gretton-Dann <matthew.gretton-dann@arm.com> + with follow-up r255433 commit. + + * gcc.c-torture/unsorted/dump-noaddr.x: Generate dump files in + tmpdir. + 2018-02-26 Carl Love <cel@us.ibm.com> Backport from mainline: commit 257747 on 2018-02-16. diff --git a/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x b/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x index d14d494570944b2be82c2575204cdbf4b15721ca..e86f36a1861fc4dc46bd449d78403f510ec4b920 100644 --- a/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x +++ b/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x @@ -9,14 +9,14 @@ proc dump_compare { src options } { # loop through all the options foreach option $option_list { - file delete -force dump1 - file mkdir dump1 + file delete -force $tmpdir/dump1 + file mkdir $tmpdir/dump1 c-torture-compile $src "$option $options -dumpbase dump1/$dumpbase -DMASK=1 -x c --param ggc-min-heapsize=1 -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr" - file delete -force dump2 - file mkdir dump2 + file delete -force $tmpdir/dump2 + file mkdir $tmpdir/dump2 c-torture-compile $src "$option $options -dumpbase dump2/$dumpbase -DMASK=2 -x c -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr" - foreach dump1 [lsort [glob -nocomplain dump1/*]] { - regsub dump1/ $dump1 dump2/ dump2 + foreach dump1 [lsort [glob -nocomplain $tmpdir/dump1/*]] { + set dump2 "$tmpdir/dump2/[file tail $dump1]" set dumptail "gcc.c-torture/unsorted/[file tail $dump1]" regsub {\.\d+((t|r|i)\.[^.]+)$} $dumptail {.*\1} dumptail set tmp [ diff "$dump1" "$dump2" ] @@ -29,8 +29,8 @@ proc dump_compare { src options } { } } } - file delete -force dump1 - file delete -force dump2 + file delete -force $tmpdir/dump1 + file delete -force $tmpdir/dump2 } dump_compare $src $options
On Tue, Dec 5, 2017 at 11:27 AM Mike Stump <mikestump@comcast.net> wrote: > > On Dec 5, 2017, at 11:11 AM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > > > > On 05/12/17 17:54, Andrew Pinski wrote: > >> On Tue, Dec 5, 2017 at 9:50 AM, Thomas Preudhomme > >> <thomas.preudhomme@foss.arm.com> wrote: > >>> Hi, > >>> > >>> dump-noaddr test FAILS when $tmpdir is not the same as the directory > >>> where runtest is called from. Note that this does not happen when > >>> running make check because tmpdir is set to srcdir. > >>> > >>> In that case, file mkdir will create the directory in the current > >>> directory while GCC is invoked from tmpdir and hence -dumpbase look > >>> for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to > >>> be relative to tmpdir which will work in all case. > >>> > >>> ChangeLog entry is as follows: > >>> > >>> *** gcc/testsuite/ChangeLog *** > >>> > >>> 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> > >>> > >>> * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base > >>> relative to tmpdir. > >>> > >>> Testing: Successfully ran unsorted.exp via make check and out of tree > >>> testing using runtest from <path>/test with tmpdir set in > >>> <path>/test/site.exp to <path>. > >>> > >>> Is this ok for stage3? > >> https://gcc.gnu.org/ml/gcc-patches/2012-06/msg01752.html > >> I don't remember where this discussion went last time. > >> Maybe this time there will be a resolution :). > > > > FWIW, I agree with Matt, creating the dump in tmpdir makes more sense. I think his patch can be simplified though because the compiler seems to be invoked from tmpdir so it can at least be omitted from the -dumpbase. > > Sounds reasonable. I've added that on top of his patch and checked that in. Let us know if it works or not. I finally got around to testing this myself and I had to revert the second patch of the set which removed $tmpdir from dumpbase option. Maybe it is the way I run the testsuite out of tree; I have been running this way since at least 2010 and the code to do the running existed before I joined Cavium/Marvell. This is how I run the testsuite (part of the Makefile): gcc.sum g++.sum gdb.sum: rm -f *.gcda mkdir -p $(basename $@) -$(RUNTEST) --tool $(basename $@) TMPDIR=`pwd`/$(basename $@) $(DGFLAGS) \ --srcdir $(SRC)/$(subst g++,gcc,$(basename $@))/testsuite \ HOSTCC=gcc HOSTCFLAGS= I don't "cd TMPDIR" before invoking runtest though so I wonder being in the TMPDIR is always true because of that. Thanks, Andrew Pinski Thanks, Andrew Pinski
On Tue, Dec 5, 2017 at 9:50 AM Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > > Hi, > > dump-noaddr test FAILS when $tmpdir is not the same as the directory > where runtest is called from. Note that this does not happen when > running make check because tmpdir is set to srcdir. > > In that case, file mkdir will create the directory in the current > directory while GCC is invoked from tmpdir and hence -dumpbase look > for dump1 and dump2 relative to tmpdir. This patch forces dumpbase to > be relative to tmpdir which will work in all case. I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107602 for this since it seems like this patch keeps on getting lost in the discussion and never being fixed fully. Thanks, Andrew > > ChangeLog entry is as follows: > > *** gcc/testsuite/ChangeLog *** > > 2017-12-05 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * gcc.c-torture/unsorted/dump-noaddr.x (dump_compare): Set dump base > relative to tmpdir. > > Testing: Successfully ran unsorted.exp via make check and out of tree > testing using runtest from <path>/test with tmpdir set in > <path>/test/site.exp to <path>. > > Is this ok for stage3? > > Best regards, > > Thomas
diff --git a/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x b/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x index d14d494570944b2be82c2575204cdbf4b15721ca..68d6c3e38325cabbdd280ecf05e663dbcda99900 100644 --- a/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x +++ b/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x @@ -11,10 +11,10 @@ proc dump_compare { src options } { foreach option $option_list { file delete -force dump1 file mkdir dump1 - c-torture-compile $src "$option $options -dumpbase dump1/$dumpbase -DMASK=1 -x c --param ggc-min-heapsize=1 -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr" + c-torture-compile $src "$option $options -dumpbase [pwd]/dump1/$dumpbase -DMASK=1 -x c --param ggc-min-heapsize=1 -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr" file delete -force dump2 file mkdir dump2 - c-torture-compile $src "$option $options -dumpbase dump2/$dumpbase -DMASK=2 -x c -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr" + c-torture-compile $src "$option $options -dumpbase [pwd]/dump2/$dumpbase -DMASK=2 -x c -fdump-ipa-all -fdump-rtl-all -fdump-tree-all -fdump-noaddr" foreach dump1 [lsort [glob -nocomplain dump1/*]] { regsub dump1/ $dump1 dump2/ dump2 set dumptail "gcc.c-torture/unsorted/[file tail $dump1]"