[PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Thomas Preudhomme]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

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?

Best regards,

Thomas

[-- Attachment #2: fix_dump-noaddr_dumpbase.patch --]
[-- Type: text/x-patch, Size: 1308 bytes --]

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]"

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[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.
>
> 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

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Thomas Preudhomme]

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

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Mike Stump]

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.

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Thomas Preudhomme]

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.
> 

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Mike Stump]

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.

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Thomas Preudhomme]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

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.
> 

[-- Attachment #2: fix_oot_dump-noaddr_testing.patch --]
[-- Type: text/x-patch, Size: 2201 bytes --]

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

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Andrew Pinski]

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

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[Mike Stump]

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.

Re: [PATCH, GCC/testsuite] Fix dump-noaddr dumpbase

[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