diff mbox series

ipa/100373 - fix emutls lowering compare-debug issue

Message ID nycvar.YFH.7.76.2105061007100.17025@elmra.sevgm.obk
State New
Headers show
Series ipa/100373 - fix emutls lowering compare-debug issue | expand

Commit Message

Richard Biener May 6, 2021, 8:08 a.m. UTC
emutls figured that tls uses in debug-insns need lowering but
that obviously has effects on code-generation as can be seen
in the following IL diff with the new testcase:

   <bb 2> [local count: 1073741824]:
-  a = 0;
+  # DEBUG BEGIN_STMT
   _4 = __builtin___emutls_get_address (&__emutls_v.b);
+  # DEBUG D#1 => *_4
+  # DEBUG d => (long int) D#1
+  # DEBUG BEGIN_STMT
+  a = 0;
+  # DEBUG BEGIN_STMT
   *_4 = 0;
   return;

where it figured the debug use of b in the original

  <bb 2> [local count: 1073741824]:
  # DEBUG BEGIN_STMT
  # DEBUG D#1 => b
  # DEBUG d => (long int) D#1
  # DEBUG BEGIN_STMT
  a = 0;

needs lowering (it maybe does when we want to produce perfect
debug but that's just bad luck).

The following patch fixes this by avoiding to create a new
emutls address when visiting debug stmts and instead resets them.
Another option might be to simply not lower debug stmt uses
but I have no way to verify actual debug info for this.

Bootstrapped and tested on x86_64-unknown-linux-gnu, Iain reports
this fixes the bootstrap-debug miscompare on darwin and also
passes testing there.

Pushed.

2021-05-05  Richard Biener  <rguenther@suse.de>

	PR ipa/100373
	* tree-emutls.c (gen_emutls_addr): Pass in whether we're
	dealing with a debug use and only query existing addresses
	if so.
	(lower_emutls_1): Avoid splitting out addresses for debug
	stmts, reset the debug stmt when we fail to find existing
	lowered addresses.
	(lower_emutls_phi_arg): Set wi.stmt.

	* gcc.dg/pr100373.c: New testcase.
---
 gcc/testsuite/gcc.dg/pr100373.c | 11 +++++++++++
 gcc/tree-emutls.c               | 17 +++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr100373.c
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/pr100373.c b/gcc/testsuite/gcc.dg/pr100373.c
new file mode 100644
index 00000000000..d4cd52a95de
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr100373.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+int a;
+_Thread_local int b;
+void c()
+{
+  long d = b;
+  a = 0;
+  b = 0;
+}
diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c
index 1c9c5d5aee1..92cb6194f21 100644
--- a/gcc/tree-emutls.c
+++ b/gcc/tree-emutls.c
@@ -394,13 +394,13 @@  struct lower_emutls_data
    Append any new computation statements required to D->SEQ.  */
 
 static tree
-gen_emutls_addr (tree decl, struct lower_emutls_data *d)
+gen_emutls_addr (tree decl, struct lower_emutls_data *d, bool for_debug)
 {
   /* Compute the address of the TLS variable with help from runtime.  */
   tls_var_data *data = tls_map->get (varpool_node::get (decl));
   tree addr = data->access;
 
-  if (addr == NULL)
+  if (addr == NULL && !for_debug)
     {
       varpool_node *cvar;
       tree cdecl;
@@ -480,7 +480,7 @@  lower_emutls_1 (tree *ptr, int *walk_subtrees, void *cb_data)
 	    *ptr = t = unshare_expr (t);
 
 	  /* If we're allowed more than just is_gimple_val, continue.  */
-	  if (!wi->val_only)
+	  if (!wi->val_only || is_gimple_debug (wi->stmt))
 	    {
 	      *walk_subtrees = 1;
 	      return NULL_TREE;
@@ -536,7 +536,15 @@  lower_emutls_1 (tree *ptr, int *walk_subtrees, void *cb_data)
       return NULL_TREE;
     }
 
-  addr = gen_emutls_addr (t, d);
+  addr = gen_emutls_addr (t, d, is_gimple_debug (wi->stmt));
+  if (!addr)
+    {
+      gimple_debug_bind_reset_value (wi->stmt);
+      update_stmt (wi->stmt);
+      wi->changed = false;
+      /* Stop walking operands.  */
+      return error_mark_node;
+    }
   if (is_addr)
     {
       /* Replace "&var" with "addr" in the statement.  */
@@ -590,6 +598,7 @@  lower_emutls_phi_arg (gphi *phi, unsigned int i,
   memset (&wi, 0, sizeof (wi));
   wi.info = d;
   wi.val_only = true;
+  wi.stmt = phi;
   walk_tree (&pd->def, lower_emutls_1, &wi, NULL);
 
   /* For normal statements, we let update_stmt do its job.  But for phi