[RFC, middlend] Fix for PR54218

[RFC, middlend] Fix for PR54218

[George Thomas]

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

Hi,

I am sending a patch which solves the debugging issue (PR 54218).

The fix is to allocate stack space only once for parameters in expand pass.

The patch is attached. Could someone suggest if its right ?

Thanks,
George

[-- Attachment #2: 54218.patch --]
[-- Type: application/octet-stream, Size: 921 bytes --]

diff -Naurp gcc/cfgexpand.c gcc/cfgexpand.c
--- gcc/cfgexpand.c	2013-01-11 16:40:46.000000000 +0530
+++ gcc/cfgexpand.c	2013-01-11 16:50:21.000000000 +0530
@@ -1568,11 +1568,14 @@ expand_used_vars (void)
 	     we don't do anything here.  But those which don't contain the
 	     default def (representing a temporary based on the parm/result)
 	     we need to allocate space just like for normal VAR_DECLs.  */
-	  if (!bitmap_bit_p (SA.partition_has_default_def, i))
-	    {
-	      expand_one_var (var, true, true);
-	      gcc_assert (SA.partition_to_pseudo[i]);
-	    }
+         if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL)
+          {
+            if (!bitmap_bit_p (SA.partition_has_default_def, i))
+              {
+                expand_one_var (var, true, true);
+                gcc_assert (SA.partition_to_pseudo[i]);
+              }
+          }
 	}
     }
   pointer_map_destroy (ssa_name_decls);

Re: [RFC, middlend] Fix for PR54218

[Andrew Pinski]

On Fri, Jan 11, 2013 at 8:17 AM, George Thomas
<georgethomas.mec@gmail.com> wrote:
> Hi,
>
> I am sending a patch which solves the debugging issue (PR 54218).
>
> The fix is to allocate stack space only once for parameters in expand pass.
>
> The patch is attached. Could someone suggest if its right ?

I have just a formatting issue:
+         if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL)
+          {
+            if (!bitmap_bit_p (SA.partition_has_default_def, i))

I think it would have been better if you had done instead:
	  if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL
             && !bitmap_bit_p (SA.partition_has_default_def, i))

So there are no other white space changes.

Also missing a changelog entry too.

Thanks,
Andrew

Re: [RFC, middlend] Fix for PR54218

[George Thomas]

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

On Fri, Jan 11, 2013 at 9:53 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Fri, Jan 11, 2013 at 8:17 AM, George Thomas
> <georgethomas.mec@gmail.com> wrote:
>> Hi,
>>
>> I am sending a patch which solves the debugging issue (PR 54218).
>>
>> The fix is to allocate stack space only once for parameters in expand pass.
>>
>> The patch is attached. Could someone suggest if its right ?
>
> I have just a formatting issue:
> +         if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL)
> +          {
> +            if (!bitmap_bit_p (SA.partition_has_default_def, i))
>
> I think it would have been better if you had done instead:
>           if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL
>              && !bitmap_bit_p (SA.partition_has_default_def, i))
>

I have attached the updated patch with the changes suggested.
Also adding a dejagnu test case to reproduce the bug.

> So there are no other white space changes.
>
> Also missing a changelog entry too.
>

I am adding the change logs below.

2013-01-11  George Thomas  <george.thomas@atmel.com>
                     Senthil Kumar Selvaraj <Senthil_Kumar.Selvaraj@atmel.com>

         PR middle-end/54218

         * gcc/cfgexpand.c (expand_used_vars ) :Added
                  a step to not allocate stack space if its a parameter

         * gcc.dg/pr54218.c : New test


Hoping that the changes are fine for trunk.

Thanks,
George

[-- Attachment #2: PR54218.patch --]
[-- Type: application/octet-stream, Size: 1429 bytes --]

diff -Naurp gcc/cfgexpand.c gcc/cfgexpand.c
--- gcc/cfgexpand.c	2013-01-11 16:40:46.000000000 +0530
+++ gcc/cfgexpand.c	2013-01-11 22:57:48.000000000 +0530
@@ -1568,11 +1568,12 @@ expand_used_vars (void)
 	     we don't do anything here.  But those which don't contain the
 	     default def (representing a temporary based on the parm/result)
 	     we need to allocate space just like for normal VAR_DECLs.  */
-	  if (!bitmap_bit_p (SA.partition_has_default_def, i))
-	    {
-	      expand_one_var (var, true, true);
-	      gcc_assert (SA.partition_to_pseudo[i]);
-	    }
+         if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL
+          && !bitmap_bit_p (SA.partition_has_default_def, i))
+           {
+             expand_one_var (var, true, true);
+             gcc_assert (SA.partition_to_pseudo[i]);
+           }
 	}
     }
   pointer_map_destroy (ssa_name_decls);
diff -Naurp gcc/testsuite/gcc.dg/pr54218.c gcc/testsuite/gcc.dg/pr54218.c
--- gcc/testsuite/gcc.dg/pr54218.c	1970-01-01 05:30:00.000000000 +0530
+++ gcc/testsuite/gcc.dg/pr54218.c	2013-01-11 22:39:07.000000000 +0530
@@ -0,0 +1,16 @@
+/* { dg-do run } */
+/* { dg-options "-g -fvar-tracking" } */
+/* { dg-skip-if "" { *-*-* }  { "*" } { "-O0" } } */
+
+void func(int p)
+{
+    p = 0; /* { dg-final { gdb-test 8 "p" "0" } } */
+    p = 32;/* { dg-final { gdb-test 8 "p" "42" } } */
+}
+
+int
+main (void)
+{
+    int local = 42;
+    func(local);
+}

Re: [RFC, middlend] Fix for PR54218

[Richard Biener]

On Fri, Jan 11, 2013 at 6:37 PM, George Thomas
<georgethomas.mec@gmail.com> wrote:
> On Fri, Jan 11, 2013 at 9:53 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Fri, Jan 11, 2013 at 8:17 AM, George Thomas
>> <georgethomas.mec@gmail.com> wrote:
>>> Hi,
>>>
>>> I am sending a patch which solves the debugging issue (PR 54218).
>>>
>>> The fix is to allocate stack space only once for parameters in expand pass.
>>>
>>> The patch is attached. Could someone suggest if its right ?
>>
>> I have just a formatting issue:
>> +         if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL)
>> +          {
>> +            if (!bitmap_bit_p (SA.partition_has_default_def, i))
>>
>> I think it would have been better if you had done instead:
>>           if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL
>>              && !bitmap_bit_p (SA.partition_has_default_def, i))
>>
>
> I have attached the updated patch with the changes suggested.
> Also adding a dejagnu test case to reproduce the bug.
>
>> So there are no other white space changes.
>>
>> Also missing a changelog entry too.
>>
>
> I am adding the change logs below.
>
> 2013-01-11  George Thomas  <george.thomas@atmel.com>
>                      Senthil Kumar Selvaraj <Senthil_Kumar.Selvaraj@atmel.com>
>
>          PR middle-end/54218
>
>          * gcc/cfgexpand.c (expand_used_vars ) :Added
>                   a step to not allocate stack space if its a parameter
>
>          * gcc.dg/pr54218.c : New test
>
>
> Hoping that the changes are fine for trunk.

Please state how you tested the patch (bootstrap and regtest on which target?)

Thanks,
Richard.

>
> Thanks,
> George

Re: [RFC, middlend] Fix for PR54218

[George Thomas]

On Mon, Jan 14, 2013 at 12:50 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Jan 11, 2013 at 6:37 PM, George Thomas
> <georgethomas.mec@gmail.com> wrote:
>> On Fri, Jan 11, 2013 at 9:53 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Fri, Jan 11, 2013 at 8:17 AM, George Thomas
>>> <georgethomas.mec@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> I am sending a patch which solves the debugging issue (PR 54218).
>>>>
>>>> The fix is to allocate stack space only once for parameters in expand pass.
>>>>
>>>> The patch is attached. Could someone suggest if its right ?
>>>
>>> I have just a formatting issue:
>>> +         if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL)
>>> +          {
>>> +            if (!bitmap_bit_p (SA.partition_has_default_def, i))
>>>
>>> I think it would have been better if you had done instead:
>>>           if (TREE_CODE (SSA_NAME_VAR (var)) != PARM_DECL
>>>              && !bitmap_bit_p (SA.partition_has_default_def, i))
>>>
>>
>> I have attached the updated patch with the changes suggested.
>> Also adding a dejagnu test case to reproduce the bug.
>>
>>> So there are no other white space changes.
>>>
>>> Also missing a changelog entry too.
>>>
>>
>> I am adding the change logs below.
>>
>> 2013-01-11  George Thomas  <george.thomas@atmel.com>
>>                      Senthil Kumar Selvaraj <Senthil_Kumar.Selvaraj@atmel.com>
>>
>>          PR middle-end/54218
>>
>>          * gcc/cfgexpand.c (expand_used_vars ) :Added
>>                   a step to not allocate stack space if its a parameter
>>
>>          * gcc.dg/pr54218.c : New test
>>
>>
>> Hoping that the changes are fine for trunk.
>
> Please state how you tested the patch (bootstrap and regtest on which target?)

I initially tested my patch only on the avr target and ran the
regressions on avr.

When I tried building the default compiler, the build is failing in
default optimisation "-g -O2".

"build/genmddeps ../../gcc-trunk-new/gcc/config/i386/i386.md"
is throwing a segmentaion fault.
I am trying to debug on why this could be happening.

The build is passing  when BOOT_CXXFLAGS is made "-g3 -O0".
The succesfully built compiler does not have the bug in it.

Also tested functions with parameters and vectors as input.

I am not sure how to debug if the issue is happening while
bootstraping the compiler itself.

Thanks,
George