diff mbox

[PR,tree-optimization/71328] Fix off-by-one error in CFG/SSA updates for backward threading

Message ID b8b23546-4e69-4771-2397-ecc41bb8c57a@redhat.com
State New
Headers show

Commit Message

Jeff Law June 3, 2016, 5:24 a.m. UTC
This bug has actually been around since the introduction of the 
FSM/backwards threader a few years ago.  It was just latent.

Essentially we need to look at the copied path for branches back into 
the path -- due to an off-by-one error, we missed the case where the 
last block in the path branches back to itself.

This results in the verification failure reported in pr71328.

I'd already had this in my tree as I'd run into it dealing with one of 
the other regressions from my recent changes.  Given it's a totally 
independent issue it seemed wise to extract and fix this one independently.

Bootstrapped on x86_64 linux and installed on the trunk.

Jeff
commit 96a568909e429b0f24d61c8a2f3dd3c183d720d7
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Jun 3 05:20:16 2016 +0000

    	PR tree-optimization/71328
    	* tree-ssa-threadupdate.c (duplicate_thread_path): Fix off-by-one
    	error when checking for a jump back onto the copied path.  */
    
    	PR tree-optimization/71328
    	* gcc.c-torture/compile/pr71328.c: New test.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@237052 138bc75d-0d04-0410-961f-82ee72b054a4

Comments

Jakub Jelinek June 3, 2016, 7:13 a.m. UTC | #1
On Thu, Jun 02, 2016 at 11:24:49PM -0600, Jeff Law wrote:
> commit 96a568909e429b0f24d61c8a2f3dd3c183d720d7
> Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Fri Jun 3 05:20:16 2016 +0000
> 
>     	PR tree-optimization/71328
>     	* tree-ssa-threadupdate.c (duplicate_thread_path): Fix off-by-one
>     	error when checking for a jump back onto the copied path.  */

The C comment termination in the ChangeLog entry is weird.

	Jakub
Jeff Law June 3, 2016, 11:09 p.m. UTC | #2
On 06/03/2016 01:13 AM, Jakub Jelinek wrote:
> On Thu, Jun 02, 2016 at 11:24:49PM -0600, Jeff Law wrote:
>> commit 96a568909e429b0f24d61c8a2f3dd3c183d720d7
>> Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
>> Date:   Fri Jun 3 05:20:16 2016 +0000
>>
>>     	PR tree-optimization/71328
>>     	* tree-ssa-threadupdate.c (duplicate_thread_path): Fix off-by-one
>>     	error when checking for a jump back onto the copied path.  */
>
> The C comment termination in the ChangeLog entry is weird.
Muscle memory...  I hit "." and naturally proceed with two spaces and a 
close comment marker ;-)

Thanks for fixing it up!

jeff
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 2a0951b..2c6f6b1 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2016-06-02  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/71328
+	* tree-ssa-threadupdate.c (duplicate_thread_path): Fix off-by-one
+	error when checking for a jump back onto the copied path.  */
+
 2016-06-02  David Malcolm  <dmalcolm@redhat.com>
 
 	* config/microblaze/microblaze.c (get_branch_target): Add return
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9f7da16..263d91b 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2016-06-02  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/71328
+	* gcc.c-torture/compile/pr71328.c: New test.
+
 2016-06-02  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
 
 	PR fortran/52393
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr71328.c b/gcc/testsuite/gcc.c-torture/compile/pr71328.c
new file mode 100644
index 0000000..aa384e8
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr71328.c
@@ -0,0 +1,16 @@ 
+
+
+int a, b, c;
+void fn1() {
+  unsigned char d = 3;
+  if (d > 11)
+  lbl_596:
+  c = 0;
+  while (!d)
+    b = b;
+  unsigned char e = e || d;
+  d = e;
+  if (a)
+    d = d || a;
+  goto lbl_596;
+}
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index 620948c..1ff007a 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -2298,11 +2298,11 @@  duplicate_thread_path (edge entry, edge exit,
 	}
 
       /* Special case the last block on the path: make sure that it does not
-	 jump back on the copied path.  */
+	 jump back on the copied path, including back to itself.  */
       if (i + 1 == n_region)
 	{
 	  FOR_EACH_EDGE (e, ei, bb->succs)
-	    if (bb_in_bbs (e->dest, region_copy, n_region - 1))
+	    if (bb_in_bbs (e->dest, region_copy, n_region))
 	      {
 		basic_block orig = get_bb_original (e->dest);
 		if (orig)