diff mbox

Restrict jump threading statement simplifier to scalar types (PR71077)

Message ID 20160818182518.7397-1-patrick@parcs.ath.cx
State New
Headers show

Commit Message

Patrick Palka Aug. 18, 2016, 6:25 p.m. UTC 
In comment #5 Yuri reports that r235653 introduces a runtime failure for
176.gcc which I guess is caused by the combining step in
simplify_control_stmt_condition_1() not behaving properly on operands of
type VECTOR_TYPE.  I'm a bit stumped as to why it mishandles
VECTOR_TYPEs because the logic should be generic enough to support them
as well.  But it was confirmed that restricting the combining step to
operands of scalar type fixes the runtime failure so here is a patch
that does this.  Does this look OK to commit after bootstrap +
regtesting on x86_64-pc-linux-gnu?

gcc/ChangeLog:

	PR tree-optimization/71077
	* tree-ssa-threadedge.c (simplify_control_stmt_condition_1):
	Perform the combining step only if the operands have an integral
	or a pointer type.
---
 gcc/tree-ssa-threadedge.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Richard Biener Aug. 18, 2016, 7:22 p.m. UTC | #1 
On August 18, 2016 8:25:18 PM GMT+02:00, Patrick Palka <patrick@parcs.ath.cx> wrote:
>In comment #5 Yuri reports that r235653 introduces a runtime failure
>for
>176.gcc which I guess is caused by the combining step in
>simplify_control_stmt_condition_1() not behaving properly on operands
>of
>type VECTOR_TYPE.  I'm a bit stumped as to why it mishandles
>VECTOR_TYPEs because the logic should be generic enough to support them
>as well.  But it was confirmed that restricting the combining step to
>operands of scalar type fixes the runtime failure so here is a patch
>that does this.  Does this look OK to commit after bootstrap +
>regtesting on x86_64-pc-linux-gnu?

Hum, I'd rather understand what is going wrong.  Can you at least isolate a testcase?

Richard.

>gcc/ChangeLog:
>
>	PR tree-optimization/71077
>	* tree-ssa-threadedge.c (simplify_control_stmt_condition_1):
>	Perform the combining step only if the operands have an integral
>	or a pointer type.
>---
> gcc/tree-ssa-threadedge.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
>index 170e456..a97c00c 100644
>--- a/gcc/tree-ssa-threadedge.c
>+++ b/gcc/tree-ssa-threadedge.c
>@@ -577,6 +577,9 @@ simplify_control_stmt_condition_1 (edge e,
>   if (handle_dominating_asserts
>       && (cond_code == EQ_EXPR || cond_code == NE_EXPR)
>       && TREE_CODE (op0) == SSA_NAME
>+      /* ??? Vector types are mishandled here.  */
>+      && (INTEGRAL_TYPE_P (TREE_TYPE (op0))
>+	  || POINTER_TYPE_P (TREE_TYPE (op0)))
>       && integer_zerop (op1))
>     {
>       gimple *def_stmt = SSA_NAME_DEF_STMT (op0);
Jeff Law Aug. 18, 2016, 7:59 p.m. UTC | #2 
On 08/18/2016 01:22 PM, Richard Biener wrote:
> On August 18, 2016 8:25:18 PM GMT+02:00, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> In comment #5 Yuri reports that r235653 introduces a runtime failure
>> for
>> 176.gcc which I guess is caused by the combining step in
>> simplify_control_stmt_condition_1() not behaving properly on operands
>> of
>> type VECTOR_TYPE.  I'm a bit stumped as to why it mishandles
>> VECTOR_TYPEs because the logic should be generic enough to support them
>> as well.  But it was confirmed that restricting the combining step to
>> operands of scalar type fixes the runtime failure so here is a patch
>> that does this.  Does this look OK to commit after bootstrap +
>> regtesting on x86_64-pc-linux-gnu?
>
> Hum, I'd rather understand what is going wrong.  Can you at least isolate a testcase?
Agreed.  And a fix like this ought to include that isolated test in the 
regression suite.

jeff
diff mbox

Patch

diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index 170e456..a97c00c 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -577,6 +577,9 @@  simplify_control_stmt_condition_1 (edge e,
   if (handle_dominating_asserts
       && (cond_code == EQ_EXPR || cond_code == NE_EXPR)
       && TREE_CODE (op0) == SSA_NAME
+      /* ??? Vector types are mishandled here.  */
+      && (INTEGRAL_TYPE_P (TREE_TYPE (op0))
+	  || POINTER_TYPE_P (TREE_TYPE (op0)))
       && integer_zerop (op1))
     {
       gimple *def_stmt = SSA_NAME_DEF_STMT (op0);