diff mbox

Clean pedantic calls and useless lvalue code in fold_cond_expr_with_comparison

Message ID VI1PR0802MB21766E0B63E27FAF9EBDC8E1E7A30@VI1PR0802MB2176.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Bin Cheng Nov. 3, 2016, 2:11 p.m. UTC 
Hi,
According to analysis given by https://gcc.gnu.org/ml/gcc/2016-10/msg00228.html, calls to pedantic_non_lvalue_loc and code handling lvalue in fold_cond_expr_with_comparison are useless now.  Given this is complicated legacy code, it may be better to change code step by step, rather than doing this cleanup together with moving simplification from fold_cond_expr_with_comparison to match.pd.  
BTW, after last cleanup of pedantic_lvalues, function pedantic_non_lvalue_loc now has nothing to do with lvalue.  It could be further cleaned up, or at least renamed into something else.  This patch doesn't do that because that depends on the answer to the question of the aforementioned message.

Bootstrap and test on x86_64 and AArch64.  Any comments?

Thanks,
bin

2016-10-27  Bin Cheng  <bin.cheng@arm.com>

	* fold-const.c (fold_cond_expr_with_comparison): Remove call
	to pedantic_non_lvalue_loc.  Remove useless code for lvalue
	where cond_expr can't be a lvalue.

Comments

Richard Biener Nov. 4, 2016, 8:40 a.m. UTC | #1 
On Thu, Nov 3, 2016 at 3:11 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> According to analysis given by https://gcc.gnu.org/ml/gcc/2016-10/msg00228.html, calls to pedantic_non_lvalue_loc and code handling lvalue in fold_cond_expr_with_comparison are useless now.  Given this is complicated legacy code, it may be better to change code step by step, rather than doing this cleanup together with moving simplification from fold_cond_expr_with_comparison to match.pd.
> BTW, after last cleanup of pedantic_lvalues, function pedantic_non_lvalue_loc now has nothing to do with lvalue.  It could be further cleaned up, or at least renamed into something else.  This patch doesn't do that because that depends on the answer to the question of the aforementioned message.
>
> Bootstrap and test on x86_64 and AArch64.  Any comments?

Ok.

Note removal of [pedantic_]non_lvalue can at most result in accepting
invalid code
where we might not have any testsuite coverage.  For the 2nd case with
/* Avoid adding NOP_EXPRs in case this is an lvalue.  */ and C++ lvalue ?:
I'm not sure we have testsuite coverage given Jason failed to add a testcase
when adding the code in r34416.

Thanks,
Richard.

> Thanks,
> bin
>
> 2016-10-27  Bin Cheng  <bin.cheng@arm.com>
>
>         * fold-const.c (fold_cond_expr_with_comparison): Remove call
>         to pedantic_non_lvalue_loc.  Remove useless code for lvalue
>         where cond_expr can't be a lvalue.
Jason Merrill Nov. 4, 2016, 12:42 p.m. UTC | #2 
On Fri, Nov 4, 2016 at 4:40 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Nov 3, 2016 at 3:11 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>> Hi,
>> According to analysis given by https://gcc.gnu.org/ml/gcc/2016-10/msg00228.html, calls to pedantic_non_lvalue_loc and code handling lvalue in fold_cond_expr_with_comparison are useless now.  Given this is complicated legacy code, it may be better to change code step by step, rather than doing this cleanup together with moving simplification from fold_cond_expr_with_comparison to match.pd.
>> BTW, after last cleanup of pedantic_lvalues, function pedantic_non_lvalue_loc now has nothing to do with lvalue.  It could be further cleaned up, or at least renamed into something else.  This patch doesn't do that because that depends on the answer to the question of the aforementioned message.
>>
>> Bootstrap and test on x86_64 and AArch64.  Any comments?
>
> Ok.
>
> Note removal of [pedantic_]non_lvalue can at most result in accepting
> invalid code where we might not have any testsuite coverage.  For the 2nd case with
> /* Avoid adding NOP_EXPRs in case this is an lvalue.  */ and C++ lvalue ?:
> I'm not sure we have testsuite coverage given Jason failed to add a testcase
> when adding the code in r34416.

Now that we're delaying folding in C++, it shouldn't matter whether
fold is lvalue-safe.

Jason
diff mbox

Patch

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 89ed89d..2bad470 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -5082,12 +5082,10 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
       case EQ_EXPR:
       case UNEQ_EXPR:
 	tem = fold_convert_loc (loc, arg1_type, arg1);
-	return pedantic_non_lvalue_loc (loc,
-				    fold_convert_loc (loc, type,
-						  negate_expr (tem)));
+	return fold_convert_loc (loc, type, negate_expr (tem));
       case NE_EXPR:
       case LTGT_EXPR:
-	return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, arg1));
+	return fold_convert_loc (loc, type, arg1);
       case UNGE_EXPR:
       case UNGT_EXPR:
 	if (flag_trapping_math)
@@ -5098,7 +5096,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 	if (TYPE_UNSIGNED (TREE_TYPE (arg1)))
 	  break;
 	tem = fold_build1_loc (loc, ABS_EXPR, TREE_TYPE (arg1), arg1);
-	return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, tem));
+	return fold_convert_loc (loc, type, tem);
       case UNLE_EXPR:
       case UNLT_EXPR:
 	if (flag_trapping_math)
@@ -5124,7 +5122,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
       && integer_zerop (arg01) && integer_zerop (arg2))
     {
       if (comp_code == NE_EXPR)
-	return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, arg1));
+	return fold_convert_loc (loc, type, arg1);
       else if (comp_code == EQ_EXPR)
 	return build_zero_cst (type);
     }
@@ -5170,20 +5168,12 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
       tree comp_op1 = arg01;
       tree comp_type = TREE_TYPE (comp_op0);
 
-      /* Avoid adding NOP_EXPRs in case this is an lvalue.  */
-      if (TYPE_MAIN_VARIANT (comp_type) == TYPE_MAIN_VARIANT (type))
-	{
-	  comp_type = type;
-	  comp_op0 = arg1;
-	  comp_op1 = arg2;
-	}
-
       switch (comp_code)
 	{
 	case EQ_EXPR:
-	  return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, arg2));
+	  return fold_convert_loc (loc, type, arg2);
 	case NE_EXPR:
-	  return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, arg1));
+	  return fold_convert_loc (loc, type, arg1);
 	case LE_EXPR:
 	case LT_EXPR:
 	case UNLE_EXPR:
@@ -5200,8 +5190,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 		    ? fold_build2_loc (loc, MIN_EXPR, comp_type, comp_op0, comp_op1)
 		    : fold_build2_loc (loc, MIN_EXPR, comp_type,
 				   comp_op1, comp_op0);
-	      return pedantic_non_lvalue_loc (loc,
-					  fold_convert_loc (loc, type, tem));
+	      return fold_convert_loc (loc, type, tem);
 	    }
 	  break;
 	case GE_EXPR:
@@ -5216,19 +5205,16 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 		    ? fold_build2_loc (loc, MAX_EXPR, comp_type, comp_op0, comp_op1)
 		    : fold_build2_loc (loc, MAX_EXPR, comp_type,
 				   comp_op1, comp_op0);
-	      return pedantic_non_lvalue_loc (loc,
-					  fold_convert_loc (loc, type, tem));
+	      return fold_convert_loc (loc, type, tem);
 	    }
 	  break;
 	case UNEQ_EXPR:
 	  if (!HONOR_NANS (arg1))
-	    return pedantic_non_lvalue_loc (loc,
-					fold_convert_loc (loc, type, arg2));
+	    return fold_convert_loc (loc, type, arg2);
 	  break;
 	case LTGT_EXPR:
 	  if (!HONOR_NANS (arg1))
-	    return pedantic_non_lvalue_loc (loc,
-					fold_convert_loc (loc, type, arg1));
+	    return fold_convert_loc (loc, type, arg1);
 	  break;
 	default:
 	  gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
@@ -5267,8 +5253,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 	    tem = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (arg00), arg00,
 				   fold_convert_loc (loc, TREE_TYPE (arg00),
 						     arg2));
-	    return pedantic_non_lvalue_loc (loc,
-					    fold_convert_loc (loc, type, tem));
+	    return fold_convert_loc (loc, type, tem);
 	  }
 	break;
 
@@ -5285,8 +5270,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 	    tem = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (arg00), arg00,
 				   fold_convert_loc (loc, TREE_TYPE (arg00),
 						     arg2));
-	    return pedantic_non_lvalue_loc (loc,
-					    fold_convert_loc (loc, type, tem));
+	    return fold_convert_loc (loc, type, tem);
 	  }
 	break;
 
@@ -5303,7 +5287,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 	    tem = fold_build2_loc (loc, MAX_EXPR, TREE_TYPE (arg00), arg00,
 				   fold_convert_loc (loc, TREE_TYPE (arg00),
 						     arg2));
-	    return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, tem));
+	    return fold_convert_loc (loc, type, tem);
 	  }
 	break;
 
@@ -5319,7 +5303,7 @@  fold_cond_expr_with_comparison (location_t loc, tree type,
 	    tem = fold_build2_loc (loc, MAX_EXPR, TREE_TYPE (arg00), arg00,
 				   fold_convert_loc (loc, TREE_TYPE (arg00),
 						     arg2));
-	    return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, tem));
+	    return fold_convert_loc (loc, type, tem);
 	  }
 	break;
       case NE_EXPR: