Bug 88001 - ASMCONS cannot handle properly UNSPEC(CONST)
Summary: ASMCONS cannot handle properly UNSPEC(CONST)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-13 09:39 UTC by Claudiu Zissulescu
Modified: 2018-12-15 12:09 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-12-11 00:00:00


Attachments
proposed patch (252 bytes, patch)
2018-12-11 14:57 UTC, Segher Boessenkool
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Claudiu Zissulescu 2018-11-13 09:39:07 UTC
ASMCONS cannot handle CONST (UNSPEC) properly, leading to a wrong
output.

I have the following rtl before asmcons pass:

(insn 8 13 9 2 (set (reg:SI 157 [ list ])
        (asm_operands:SI ("") ("=g") 0 [
                (const:SI (unspec:SI [
                            (symbol_ref:SI ("c_const") [flags 0x2]  <var_decl 0x7f6735ad25a0 c_const>)
                        ] ARC_UNSPEC_GOTOFFPC))
            ]
             [
                (asm_input:SI ("0") ../t02.c:9)
            ]
             [] ../t02.c:9)) ../t02.c:9 -1
     (nil))

Asmcons pass leads to this:

(insn 13 3 8 2 (set (reg:SI 157 [ list ])
        (const:SI (unspec:SI [
                    (symbol_ref:SI ("c_const") [flags 0x2]  <var_decl 0x7fd69f6365a0 c_const>)
                ] ARC_UNSPEC_GOTOFFPC))) ../t02.c:9 -1
     (nil))
(insn 8 13 9 2 (set (reg:SI 157 [ list ])
        (asm_operands:SI ("") ("=g") 0 [
                (const:SI (unspec:SI [
                            (symbol_ref:SI ("c_const") [flags 0x2]  <var_decl 0x7fd69f6365a0 c_const>)
                        ] ARC_UNSPEC_GOTOFFPC))
            ]
             [
                (asm_input:SI ("0") ../t02.c:9)
            ]
             [] ../t02.c:9)) ../t02.c:9 -1
     (nil))

Which will lead latter on to an ICE when we verify the rtx sharing.

My test program is this one, and it needs to be compiled for ARC backend with the following options: -mcpu=archs -O2 -fpic

typedef void (*func_ptr) (void);
static func_ptr __DTOR_LIST__[1] = { (func_ptr)(-1) };

void foo (int a)
{
  func_ptr *dtor_list;
  __asm ("" : "=g" (dtor_list) : "0" (__DTOR_LIST__));
  dtor_list[a]();
}
Comment 1 Claudiu Zissulescu 2018-11-13 09:44:45 UTC
My solution, on a side branch, is this patch, but we need it to run also for mainline gcc as we cannot build glibc or uclibc toolchains. Any help is appreciated.

---
 gcc/function.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/gcc/function.c b/gcc/function.c
index 302438323c8..36227f77074 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -6374,6 +6374,37 @@ make_pass_thread_prologue_and_epilogue (gcc::context *ctxt)
 }
 
 
+/* Helper match_asm_constraints_1.  */
+static int
+constant_overlap_mentioned_p (const_rtx x, const_rtx in)
+{
+  const char *fmt;
+  int i, j;
+
+  if (CONST_INT_P (in))
+    return 0;
+
+  if (!CONSTANT_P (in))
+    return 0;
+
+  if (x == 0)
+    return 0;
+
+  if (x == in)
+    return 1;
+
+  fmt = GET_RTX_FORMAT (GET_CODE (x));
+  for (i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0; i--)
+    {
+      if (fmt[i] == 'e')
+	 return constant_overlap_mentioned_p (XEXP (x, i), in);
+      else if (fmt[i] == 'E')
+	for (j = XVECLEN (x, i) - 1; j >= 0; j--)
+	  return constant_overlap_mentioned_p (XVECEXP (x, i, j), in);
+    }
+  return 0;
+}
+
 /* This mini-pass fixes fall-out from SSA in asm statements that have
    in-out constraints.  Say you start with
 
@@ -6509,7 +6540,8 @@ match_asm_constraints_1 (rtx_insn *insn, rtx *p_sets, int noutputs)
 	  SET_DEST (p_sets[j]) = replace_rtx (SET_DEST (p_sets[j]),
 					      input, output);
       for (j = 0; j < ninputs; j++)
-	if (reg_overlap_mentioned_p (input, RTVEC_ELT (inputs, j)))
+	if (reg_overlap_mentioned_p (input, RTVEC_ELT (inputs, j))
+	    || constant_overlap_mentioned_p (RTVEC_ELT (inputs, j), input))
 	  RTVEC_ELT (inputs, j) = replace_rtx (RTVEC_ELT (inputs, j),
 					       input, output);
 
-- 
2.19.1
Comment 2 Segher Boessenkool 2018-12-11 11:53:18 UTC
Ah yes, I had release checking there.  Confirmed.
Comment 3 Segher Boessenkool 2018-12-11 14:57:44 UTC
Created attachment 45210 [details]
proposed patch
Comment 4 Segher Boessenkool 2018-12-11 14:59:12 UTC
Claudiu: could you test that patch please?  (On the real thing, not just
this testcase :-) )
Comment 5 Vineet Gupta 2018-12-12 06:13:55 UTC
Hi, I'm the ARC glibc maintainer (and desperate for this fix).

The proposed (one liner) patch seems to fix the compiler assert for sue. I'm still running more tests to make sure it is functional but looks promising.
Comment 6 Claudiu Zissulescu 2018-12-12 06:30:45 UTC
The dejagnu tests for ARC look alright. No extra failures.
Comment 7 Segher Boessenkool 2018-12-12 20:23:08 UTC
Patch posted at https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00865.html .
Comment 8 Segher Boessenkool 2018-12-14 08:30:18 UTC
Author: segher
Date: Fri Dec 14 08:29:34 2018
New Revision: 267122

URL: https://gcc.gnu.org/viewcvs?rev=267122&root=gcc&view=rev
Log:
match_asm_constraints: Use copy_rtx where needed (PR88001)

The new insn here (temporarily) illegally shares RTL.  This fixes it.


	PR rtl-optimization/88001
	* function.c (match_asm_constraints_1): Don't invalidly share RTL.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/function.c
Comment 9 Vineet Gupta 2018-12-14 21:09:13 UTC
Can this be stable backported to gcc-8-branch as well.
glibc folks use that branch for their regular smoke testing and without that ARC tools don't even build.
Comment 10 Segher Boessenkool 2018-12-15 01:27:42 UTC
Sure this can be backported...  But can you fill in known-to-{work,fail}
then please?  Thanks.
Comment 11 Vineet Gupta 2018-12-15 05:28:27 UTC
Sure, but how can I ? if i click the "known to work" field it takes me to help.

The issue certainly with gcc-8-branch for ARC and presumably also with tip/trunk.
Comment 12 Segher Boessenkool 2018-12-15 12:05:40 UTC
Author: segher
Date: Sat Dec 15 12:05:08 2018
New Revision: 267171

URL: https://gcc.gnu.org/viewcvs?rev=267171&root=gcc&view=rev
Log:
	Backport from trunk
	2018-12-14  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/88001
	* function.c (match_asm_constraints_1): Don't invalidly share RTL.

Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/function.c
Comment 13 Segher Boessenkool 2018-12-15 12:08:14 UTC
Author: segher
Date: Sat Dec 15 12:07:42 2018
New Revision: 267172

URL: https://gcc.gnu.org/viewcvs?rev=267172&root=gcc&view=rev
Log:
	Backport from trunk
	2018-12-14  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/88001
	* function.c (match_asm_constraints_1): Don't invalidly share RTL.

Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/function.c
Comment 14 Segher Boessenkool 2018-12-15 12:09:14 UTC
Ah, you don't have permission to edit the field.  I backported it to GCC 8
and GCC 7; closing now.