diff mbox

[IPA,ICF] Fix PR63664, PR63574 (segfault in ipa-icf pass)

Message ID 545115BD.8040709@suse.cz
State New
Headers show

Commit Message

Martin Liška Oct. 29, 2014, 4:28 p.m. UTC 
On 10/29/2014 03:07 PM, Ilya Enkovich wrote:
> 2014-10-29 17:01 GMT+03:00 Martin Liška <mliska@suse.cz>:
>> On 10/29/2014 02:45 PM, Ilya Enkovich wrote:
>>>
>>> On 29 Oct 10:34, Richard Biener wrote:
>>>>
>>>> On Tue, Oct 28, 2014 at 5:14 PM, Ilya Enkovich <enkovich.gnu@gmail.com>
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> This patch fixes PR63664 and PR63574.  Problem is in NULL types for
>>>>> labels not handled by ICF properly.  I assume it is OK for labels to have
>>>>> NULL type and added check into ICF rather then fixed label generation.
>>>>>
>>>>> Bootstrapped and checked on linux-x86_64.  OK for trunk?
>>>>
>>>>
>>>> Instead it shouldn't be called for labels instead.
>>>>
>>>> Richard.
>>>>
>>>
>>> Here is a version which doesn't compare types for labels.  Is is OK?
>>
>>
>> Hello.
>>
>> I've been just testing a patch, where the pass does not call compare_operand
>> for gimple labels.
>> As the pass creates mapping between labels and basic blocks, such comparison
>> will not be necessary.
>
> OK.  That would be better.

Hello.

Following patch fixes PR ipa/63574, where IPA ICF calls unnecessary compare_operand for LABEL_DECLs.
Patch has been tested on x86_64-linux-pc without any regression and boostrap works correctly.

Ready for thunk?
Thanks,
Martin


>
> Thanks,
> Ilya
>
>>
>> Thanks,
>> Martin
>>
>>
>>>
>>> Bootstrapped and checked on linux-x86_64.
>>>
>>> Thanks,
>>> Ilya
>>> --
>>> gcc/
>>>
>>> 2014-10-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>          PR ipa/63664
>>>          PR bootstrap/63574
>>>          * ipa-icf-gimple.c (func_checker::compatible_types_p): Assert for
>>> null
>>>          args.
>>>          (func_checker::compare_operand): Don't compare types for labels.
>>>
>>> gcc/testsuite/
>>>
>>> 2014-10-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>          PR ipa/63664
>>>          * gcc.dg/ipa/pr63664.C: New.
>>>
>>>
>>> diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
>>> index 1369b74..094e8ab 100644
>>> --- a/gcc/ipa-icf-gimple.c
>>> +++ b/gcc/ipa-icf-gimple.c
>>> @@ -169,6 +169,9 @@ bool func_checker::compatible_types_p (tree t1, tree
>>> t2,
>>>                                         bool compare_polymorphic,
>>>                                         bool first_argument)
>>>    {
>>> +  gcc_assert (t1);
>>> +  gcc_assert (t2);
>>> +
>>>      if (TREE_CODE (t1) != TREE_CODE (t2))
>>>        return return_false_with_msg ("different tree types");
>>>
>>> @@ -214,11 +217,15 @@ func_checker::compare_operand (tree t1, tree t2)
>>>      else if (!t1 || !t2)
>>>        return false;
>>>
>>> -  tree tt1 = TREE_TYPE (t1);
>>> -  tree tt2 = TREE_TYPE (t2);
>>> +  if (TREE_CODE (t1) != LABEL_DECL
>>> +      && TREE_CODE (t2) != LABEL_DECL)
>>> +    {
>>> +      tree tt1 = TREE_TYPE (t1);
>>> +      tree tt2 = TREE_TYPE (t2);
>>>
>>> -  if (!func_checker::compatible_types_p (tt1, tt2))
>>> -    return false;
>>> +      if (!func_checker::compatible_types_p (tt1, tt2))
>>> +       return false;
>>> +    }
>>>
>>>      base1 = get_addr_base_and_unit_offset (t1, &offset1);
>>>      base2 = get_addr_base_and_unit_offset (t2, &offset2);
>>> diff --git a/gcc/testsuite/gcc.dg/ipa/pr63664.C
>>> b/gcc/testsuite/gcc.dg/ipa/pr63664.C
>>> new file mode 100644
>>> index 0000000..31d96d4
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/ipa/pr63664.C
>>> @@ -0,0 +1,43 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O2" } */
>>> +
>>> +class test {
>>> + public:
>>> +  test (int val, int *p)
>>> +    {
>>> +      int_val = *p;
>>> +      bool_val = (val != int_val);
>>> +    }
>>> +
>>> +  ~test ()
>>> +    {
>>> +      if (!bool_val)
>>> +       return;
>>> +    }
>>> +
>>> +  int get_int_val () const { return int_val; }
>>> +
>>> + private:
>>> +  bool bool_val;
>>> +  int int_val;
>>> +};
>>> +
>>> +static int __attribute__ ((noinline))
>>> +f1 (int i, int *p)
>>> +{
>>> +  test obj (i, p);
>>> +  return obj.get_int_val ();
>>> +}
>>> +
>>> +static int __attribute__ ((noinline))
>>> +f2 (int i, int *p)
>>> +{
>>> +  test obj (i, p);
>>> +  return obj.get_int_val ();
>>> +}
>>> +
>>> +int
>>> +f (int i, int *p)
>>> +{
>>> +  return f1 (i, p) + f2 (i, p);
>>> +}
>>>
>>
gcc/testsuite/ChangeLog:

2014-10-29  Martin Liska  <mliska@suse.cz>

	* g++.dg/ipa/pr63574.C: New test.


gcc/ChangeLog:

2014-10-29  Martin Liska  <mliska@suse.cz>

	* ipa-icf-gimple.c (func_checker::compare_variable_decl):
	(func_checker::parse_labels):
	(func_checker::compare_gimple_label):
	* ipa-icf-gimple.h:

Comments

Jeff Law Oct. 29, 2014, 8:43 p.m. UTC | #1 
On 10/29/14 10:28, Martin Liška wrote:
>
>
> PR63574.changelog
>
>
> gcc/testsuite/ChangeLog:
>
> 2014-10-29  Martin Liska<mliska@suse.cz>
>
> 	* g++.dg/ipa/pr63574.C: New test.
>
>
> gcc/ChangeLog:
>
> 2014-10-29  Martin Liska<mliska@suse.cz>
>
> 	* ipa-icf-gimple.c (func_checker::compare_variable_decl):
> 	(func_checker::parse_labels):
> 	(func_checker::compare_gimple_label):
> 	* ipa-icf-gimple.h:
ChangeLog needs to be completed :-)

With that fixed, OK for the trunk.

Jeff
diff mbox

Patch

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index d3f3795..ecb9667 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -527,6 +527,10 @@  func_checker::compare_variable_decl (tree t1, tree t2)
   return return_with_debug (ret);
 }
 
+
+/* Function visits all gimple labels and creates corresponding
+   mapping between basic blocks and labels.  */
+
 void
 func_checker::parse_labels (sem_bb *bb)
 {
@@ -765,7 +769,8 @@  func_checker::compare_gimple_label (gimple g1, gimple g2)
   if (FORCED_LABEL (t1) || FORCED_LABEL (t2))
     return return_false_with_msg ("FORCED_LABEL");
 
-  return compare_tree_ssa_label (t1, t2);
+  /* As the pass build BB to label mapping, no further check is needed.  */
+  return true;
 }
 
 /* Verifies for given GIMPLEs S1 and S2 that
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index 8487a2a..5811bd1 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -145,6 +145,8 @@  public:
   /* Memory release routine.  */
   ~func_checker();
 
+  /* Function visits all gimple labels and creates corresponding
+     mapping between basic blocks and labels.  */
   void parse_labels (sem_bb *bb);
 
   /* Basic block equivalence comparison function that returns true if
diff --git a/gcc/testsuite/g++.dg/ipa/pr63574.C b/gcc/testsuite/g++.dg/ipa/pr63574.C
new file mode 100644
index 0000000..59b82d5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr63574.C
@@ -0,0 +1,47 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+class test
+{
+public:
+  test (int val, int *p)
+  {
+    int_val = *p;
+    bool_val = (val != int_val);
+  }
+
+  ~test ()
+  {
+    if (!bool_val)
+      return;
+  }
+
+  int get_int_val () const
+  {
+    return int_val;
+  }
+
+private:
+  bool bool_val;
+  int int_val;
+};
+
+static int __attribute__ ((noinline))
+f1 (int i, int *p)
+{
+  test obj (i, p);
+  return obj.get_int_val ();
+}
+
+static int __attribute__ ((noinline))
+f2 (int i, int *p)
+{
+  test obj (i, p);
+  return obj.get_int_val ();
+}
+
+int
+f (int i, int *p)
+{
+  return f1 (i, p) + f2 (i, p);
+}