public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH GCC]Clean pedantic calls and useless lvalue code in fold_cond_expr_with_comparison
@ 2016-11-03 14:11 Bin Cheng
  2016-11-04  8:40 ` Richard Biener
  0 siblings, 1 reply; 3+ messages in thread
From: Bin Cheng @ 2016-11-03 14:11 UTC (permalink / raw)
  To: gcc-patches; +Cc: nd

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

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.

[-- Attachment #2: clean-pedantic-call-for-fold_cond_expr-20161101.txt --]
[-- Type: text/plain, Size: 4958 bytes --]

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:

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH GCC]Clean pedantic calls and useless lvalue code in fold_cond_expr_with_comparison
  2016-11-03 14:11 [PATCH GCC]Clean pedantic calls and useless lvalue code in fold_cond_expr_with_comparison Bin Cheng
@ 2016-11-04  8:40 ` Richard Biener
  2016-11-04 12:43   ` Jason Merrill
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2016-11-04  8:40 UTC (permalink / raw)
  To: Bin Cheng, Jason Merrill; +Cc: gcc-patches, nd

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH GCC]Clean pedantic calls and useless lvalue code in fold_cond_expr_with_comparison
  2016-11-04  8:40 ` Richard Biener
@ 2016-11-04 12:43   ` Jason Merrill
  0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2016-11-04 12:43 UTC (permalink / raw)
  To: Richard Biener; +Cc: Bin Cheng, gcc-patches, nd

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-11-04 12:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-03 14:11 [PATCH GCC]Clean pedantic calls and useless lvalue code in fold_cond_expr_with_comparison Bin Cheng
2016-11-04  8:40 ` Richard Biener
2016-11-04 12:43   ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).