This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][PR middle-end/59127] Fix Ada bootstrap on x86_64-unknown-linux-gnu



This patch fixes two issues, the most important issue is the related to the Ada build failures on the trunk.

When non-call-exceptions is on, most memory references potentially throw. As a result those statements end basic blocks. This causes checking failures when the __builtin_trap is placed immediately after the memory reference because we find the memory reference in the middle of a basic block.

While I think we could support this with some more work, it just doesn't seem worth the effort. It certainly isn't something that's occurring with any regularity AFAICT when buliding the Ada compiler/runtime system.

It's easiest to just disallow optimization when the statement that triggers undefined behaviour ends a block -- with the exception of GIMPLE_RETURN. That captures the key issue, namely that the code is not currently prepared to have the trap in a separate block from the statement triggering undefined behaviour.

I didn't make a testcase for this failure because it triggers during bootstrapping Ada and my Ada-fu has severely eroded since the 80s when I was forced to learn Ada.

The second issue is when a block has outgoing abnormal edges, out of an abundance of caution, we should simply not apply the optimization. That may be a bit too cautious, but it's clearly the safe thing to do. I don't have a testcase for this.

In addition to the usual bootstrap & regression test on x86_64-unknown-linux-gnu, this patch fixes the Ada bootstrap on x86_64-unknown-linux-gnu and shows no regressions in the Ada test suite when compared to a compiler with the optimization totally disabled. ie, a poor mans regression test for Ada given that Ada wasn't bootstrapping w/o this patch.

Applied to the trunk.

Jeff

ps. Obviously the next thing to verify is that Ada is bootstrapping on Itanic, but there's other potential issues on Itanic that might get in the way.

	* basic-block.h (has_abnormal_outgoing_edge_p): Moved here from...
	* tree-inline.c (has_abnormal_outgoing_edge_p): Remove.
	* gimple-ssa-isolate-paths.c: Include tree-cfg.h.
	(find_implicit_erroneous_behaviour): If a block has abnormal outgoing
	edges, then ignore it.  If the statement exhibiting erroneous
	behaviour ends basic blocks, with the exception of GIMPLE_RETURNs,
	then we can not optimize.
	(find_explicit_erroneous_behaviour): Likewise.

 
diff --git a/gcc/basic-block.h b/gcc/basic-block.h
index 9c28f14..b7e3b50 100644
--- a/gcc/basic-block.h
+++ b/gcc/basic-block.h
@@ -1008,4 +1008,19 @@ inverse_probability (int prob1)
   check_probability (prob1);
   return REG_BR_PROB_BASE - prob1;
 }
+
+/* Return true if BB has at least one abnormal outgoing edge.  */
+
+static inline bool
+has_abnormal_outgoing_edge_p (basic_block bb)
+{
+  edge e;
+  edge_iterator ei;
+
+  FOR_EACH_EDGE (e, ei, bb->succs)
+    if (e->flags & EDGE_ABNORMAL)
+      return true;
+
+  return false;
+}
 #endif /* GCC_BASIC_BLOCK_H */
diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 108b98e..66c13f4 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "ssa-iterators.h"
 #include "cfgloop.h"
 #include "tree-pass.h"
+#include "tree-cfg.h"
 
 
 static bool cfg_altered;
@@ -215,6 +216,17 @@ find_implicit_erroneous_behaviour (void)
     {
       gimple_stmt_iterator si;
 
+      /* Out of an abundance of caution, do not isolate paths to a
+	 block where the block has any abnormal outgoing edges.
+
+	 We might be able to relax this in the future.  We have to detect
+	 when we have to split the block with the NULL dereference and
+	 the trap we insert.  We have to preserve abnormal edges out
+	 of the isolated block which in turn means updating PHIs at
+	 the targets of those abnormal outgoing edges.  */
+      if (has_abnormal_outgoing_edge_p (bb))
+	continue;
+
       /* First look for a PHI which sets a pointer to NULL and which
  	 is then dereferenced within BB.  This is somewhat overly
 	 conservative, but probably catches most of the interesting
@@ -256,8 +268,15 @@ find_implicit_erroneous_behaviour (void)
 	        {
 	          /* We only care about uses in BB.  Catching cases in
 		     in other blocks would require more complex path
-		     isolation code.  */
-		  if (gimple_bb (use_stmt) != bb)
+		     isolation code. 
+
+		     If the statement must end a block and is not a
+		     GIMPLE_RETURN, then additional work would be
+		     necessary to isolate the path.  Just punt it for
+		     now.  */
+		  if (gimple_bb (use_stmt) != bb
+		      || (stmt_ends_bb_p (use_stmt)
+			  && gimple_code (use_stmt) != GIMPLE_RETURN))
 		    continue;
 
 		  if (infer_nonnull_range (use_stmt, lhs))
@@ -289,6 +308,17 @@ find_explicit_erroneous_behaviour (void)
     {
       gimple_stmt_iterator si;
 
+      /* Out of an abundance of caution, do not isolate paths to a
+	 block where the block has any abnormal outgoing edges.
+
+	 We might be able to relax this in the future.  We have to detect
+	 when we have to split the block with the NULL dereference and
+	 the trap we insert.  We have to preserve abnormal edges out
+	 of the isolated block which in turn means updating PHIs at
+	 the targets of those abnormal outgoing edges.  */
+      if (has_abnormal_outgoing_edge_p (bb))
+	continue;
+
       /* Now look at the statements in the block and see if any of
 	 them explicitly dereference a NULL pointer.  This happens
 	 because of jump threading and constant propagation.  */
@@ -299,7 +329,8 @@ find_explicit_erroneous_behaviour (void)
 	  /* By passing null_pointer_node, we can use infer_nonnull_range
 	     to detect explicit NULL pointer dereferences and other uses
 	     where a non-NULL value is required.  */
-	  if (infer_nonnull_range (stmt, null_pointer_node))
+	  if ((!stmt_ends_bb_p (stmt) || gimple_code (stmt) == GIMPLE_RETURN)
+	      && infer_nonnull_range (stmt, null_pointer_node))
 	    {
 	      insert_trap_and_remove_trailing_statements (&si,
 							  null_pointer_node);
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index fe5c0cb..08c4541 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4504,21 +4504,6 @@ fold_marked_statements (int first, struct pointer_set_t *statements)
       }
 }
 
-/* Return true if BB has at least one abnormal outgoing edge.  */
-
-static inline bool
-has_abnormal_outgoing_edge_p (basic_block bb)
-{
-  edge e;
-  edge_iterator ei;
-
-  FOR_EACH_EDGE (e, ei, bb->succs)
-    if (e->flags & EDGE_ABNORMAL)
-      return true;
-
-  return false;
-}
-
 /* Expand calls to inline functions in the body of FN.  */
 
 unsigned int

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]