diff mbox

Skip ubsan internal fns in tail-merge

Message ID 56DD4E9A.3030009@mentor.com
State New
Headers show

Commit Message

Tom de Vries March 7, 2016, 9:49 a.m. UTC 
[ was: Re: [PATCH 3/3] Fix ubsan tests by disabling of an optimization. ]

On 10/07/15 22:11, Jeff Law wrote:
> On 07/10/2015 02:19 AM, Richard Biener wrote:
>>
>> But the warning on the "bogus" line will still be warranted, so user
>> goes and
>> fixes it.
> But when the user gets the "bogus" line, he may look at the code and
> determine that the reported line can't possibly be executed -- so they
> get confused, assume the warning is bogus and ignore it.
>
>
>
>   Then tail-merge no longer applies and he gets the warning on the
>> other warranted line.
> That assumes that we'd get to both paths.  We may not.  The paths may be
> totally independent.
>

IIUC, the conclusion of this discussion (starting at 
https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00758.html ) is that we 
want to disable tail-merge for ubsan internal functions?

This patch implements that.

Should the asan internal function (ASAN_CHECK) be handled in the same 
way, as suggested here ( 
https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00776.html )?

OK for stage4 trunk if bootstrap and reg-test succeeds?

Thanks,
- Tom

Comments

Jakub Jelinek March 7, 2016, 10:01 a.m. UTC | #1 
On Mon, Mar 07, 2016 at 10:49:14AM +0100, Tom de Vries wrote:
> This patch implements that.
> 
> Should the asan internal function (ASAN_CHECK) be handled in the same way,
> as suggested here ( https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00776.html
> )?
> 
> OK for stage4 trunk if bootstrap and reg-test succeeds?

Please certainly add ASAN_CHECK to the list.
Perhaps more precise would be to allow merging them, but only if
gimple_location (stmt1) == gimple_location (stmt2).

Also, I wonder why you need to check both stmts individually, wouldn't it be
better (both for the is_tm_ending and the ubsan/asan internal calls) to
first call gimple_equal_p and only if it says the two are equal, check
is_tm_ending (only on one stmt, if they are equal, then I hope is_tm_ending
is necessarily equal for both), and similarly for the ubsan/asan ifns
compare the gimple_location?

> Skip ubsan internal fns in tail-merge
> 
> 2016-03-07  Tom de Vries  <tom@codesourcery.com>
> 
> 	* tree-ssa-tail-merge.c (merge_stmt_p): New function, factored out
> 	of ...
> 	(find_duplicate): ... here.
> 	(merge_stmt_p): Return false for ubsan functions.

	Jakub
diff mbox

Patch

Skip ubsan internal fns in tail-merge

2016-03-07  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-tail-merge.c (merge_stmt_p): New function, factored out
	of ...
	(find_duplicate): ... here.
	(merge_stmt_p): Return false for ubsan functions.

---
 gcc/tree-ssa-tail-merge.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
index 5d32790..7f6e922 100644
--- a/gcc/tree-ssa-tail-merge.c
+++ b/gcc/tree-ssa-tail-merge.c
@@ -1207,6 +1207,33 @@  gsi_advance_bw_nondebug_nonlocal (gimple_stmt_iterator *gsi, tree *vuse,
     }
 }
 
+/* Return true if STMT is allowed to be merged.  */
+
+static bool
+merge_stmt_p (gimple *stmt)
+{
+  if (is_tm_ending (stmt))
+    return false;
+
+  if (is_gimple_call (stmt)
+      && gimple_call_internal_p (stmt))
+    switch (gimple_call_internal_fn (stmt))
+      {
+      case IFN_UBSAN_NULL:
+      case IFN_UBSAN_BOUNDS:
+      case IFN_UBSAN_VPTR:
+      case IFN_UBSAN_CHECK_ADD:
+      case IFN_UBSAN_CHECK_SUB:
+      case IFN_UBSAN_CHECK_MUL:
+      case IFN_UBSAN_OBJECT_SIZE:
+	return false;
+      default:
+	break;
+      }
+
+  return true;
+}
+
 /* Determines whether BB1 and BB2 (members of same_succ) are duplicates.  If so,
    clusters them.  */
 
@@ -1229,8 +1256,8 @@  find_duplicate (same_succ *same_succ, basic_block bb1, basic_block bb2)
       /* What could be better than this here is to blacklist the bb
 	 containing the stmt, when encountering the stmt f.i. in
 	 same_succ_hash.  */
-      if (is_tm_ending (stmt1)
-	  || is_tm_ending (stmt2))
+      if (!merge_stmt_p (stmt1)
+	  || !merge_stmt_p (stmt2))
 	return;
 
       if (!gimple_equal_p (same_succ, stmt1, stmt2))