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 rtl-optimization/81308] Conditionally cleanup the CFG after switch conversion


This patch fixes the second testcase in 81308 and the duplicate in 83724.

For those cases we have a switch statement where one or more case labels
are marked as __builtin_unreachable.

Switch conversion calls group_case_labels which can drop the edges from
the switch to the case labels that are marked as __builtin_unreachable.

That leaves those blocks unreachable and thus we again trigger the
assertion failure in the dominance code.

My solution here is again to note if a change was made that ought to
trigger a CFG cleanup (group_case_labels returns a suitable boolean).
If such a change was made then the pass returns TODO_cleanup_cfg.

Bootstrapped and regression tested on x86_64.  OK for the trunk?

Jeff

	PR rtl-optimizatin/81308
	* tree-switch-conversion.c (cfg_altered): New file scoped static.
	(process_switch): If group_case_labels makes a change, then set
	cfg_altered.
	(pass_convert_switch::execute): If a switch is converted, then
	set cfg_altered.  Return TODO_cfg_cleanup if cfg_altered is true.

	
	PR rtl-optimizatin/81308
	* g++.dg/pr81308-1.C: New test.
	* g++.dg/pr81308-2.C: New test.

diff --git a/gcc/testsuite/g++.dg/pr81308-1.C b/gcc/testsuite/g++.dg/pr81308-1.C
new file mode 100644
index 0000000..508372b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr81308-1.C
@@ -0,0 +1,67 @@
+/* { dg-do compile } */
+/* { dg-options "-w -O2 -fno-exceptions -std=c++11 -fpermissive" } */
+
+namespace a {
+template <typename b, b c> struct d { static constexpr b e = c; };
+template <typename> struct f : d<bool, __is_trivially_copyable(int)> {};
+}
+typedef long g;
+template <typename> struct h { static const bool e = a::f<int>::e; };
+namespace a {
+template <typename> struct ah;
+template <typename> class ai;
+}
+class i {
+public:
+  operator[](long) const {}
+};
+template <typename, int> class am : public i {};
+class an;
+class k : public am<a::ai<an>, h<a::ai<a::ah<an>>>::e> {};
+class l {
+public:
+  aq();
+};
+class ar extern as;
+typedef k at;
+class m {
+  virtual bool av(int, unsigned &, at &, int &, g &, bool);
+};
+class ar {
+public:
+  typedef m *aw(const &, int &, const &, const &);
+};
+struct ax {
+  static ay(ar::aw);
+};
+template <class az> struct n {
+  n(ar) { ax::ay(ba); }
+  static m *ba(const &bb, int &bc, const &bd, const &be) { az(bb, bc, bd, be); }
+};
+namespace {
+class G : m {
+  unsigned bi(const at &, l &);
+  bool av(int, unsigned &, at &, int &, g &, bool);
+
+public:
+  G(const, int, const, const) {}
+};
+}
+bool G::av(int, unsigned &, at &bl, int &, g &, bool) {
+  l bo;
+  bi(bl, bo);
+}
+o() { n<G> bp(as); }
+namespace {
+enum { bq, br };
+}
+unsigned G::bi(const at &bl, l &bo) {
+  unsigned bs;
+  for (char *j;; j += 2)
+    switch (*j) {
+    case bq:
+      bl[bs];
+    case br:
+      bo.aq();
+    }
+}
diff --git a/gcc/testsuite/g++.dg/pr81308-2.C b/gcc/testsuite/g++.dg/pr81308-2.C
new file mode 100644
index 0000000..97e3409
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr81308-2.C
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-w -O2" } */
+
+struct A {
+  int operator[](int) const {}
+};
+struct B {
+  void m_fn1();
+};
+struct C {
+  virtual bool m_fn2(int, unsigned &, A &, int &, unsigned long &, bool);
+};
+template <class MCAsmParserImpl> struct D {
+  D(int) { MCAsmParserImpl(0, 0, 0, 0); }
+};
+int a;
+namespace {
+struct F : C {
+  bool m_fn2(int, unsigned &, A &, int &, unsigned long &, bool);
+  unsigned m_fn3(const A &, B &);
+  F(int, int, int, int) {}
+};
+}
+bool F::m_fn2(int, unsigned &, A &p3, int &, unsigned long &, bool) {
+  B b;
+  m_fn3(p3, b);
+}
+void fn1() { D<F>(0); }
+unsigned F::m_fn3(const A &p1, B &p2) {
+  for (int *p;; p++)
+    switch (*p) {
+    case 0:
+      p1[a];
+    case 1:
+      p2.m_fn1();
+    }
+}
+
diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index fdec59e..b384e4d 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -60,6 +60,10 @@ Software Foundation, 51 Franklin Street, Fifth Floor, Boston, MA
 	  targetm.case_values_threshold(), or be its own param.  */
 #define MAX_CASE_BIT_TESTS  3
 
+/* Track whether or not we have altered the CFG and thus may need to
+   cleanup the CFG when complete.  */
+bool cfg_altered;
+
 /* Split the basic block at the statement pointed to by GSIP, and insert
    a branch to the target basic block of E_TRUE conditional on tree
    expression COND.
@@ -1492,7 +1496,7 @@ process_switch (gswitch *swtch)
 
   /* Group case labels so that we get the right results from the heuristics
      that decide on the code generation approach for this switch.  */
-  group_case_labels_stmt (swtch);
+  cfg_altered |= group_case_labels_stmt (swtch);
 
   /* If this switch is now a degenerate case with only a default label,
      there is nothing left for us to do.   */
@@ -1605,6 +1609,7 @@ pass_convert_switch::execute (function *fun)
 {
   basic_block bb;
 
+  cfg_altered = false;
   FOR_EACH_BB_FN (bb, fun)
   {
     const char *failure_reason;
@@ -1625,6 +1630,7 @@ pass_convert_switch::execute (function *fun)
 	failure_reason = process_switch (as_a <gswitch *> (stmt));
 	if (! failure_reason)
 	  {
+	    cfg_altered = true;
 	    if (dump_file)
 	      {
 		fputs ("Switch converted\n", dump_file);
@@ -1648,7 +1654,7 @@ pass_convert_switch::execute (function *fun)
       }
   }
 
-  return 0;
+  return cfg_altered ? TODO_cleanup_cfg : 0;
 }
 
 } // anon namespace

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