Patchwork PR 52125: Detecting which operands are used in an asm

login
register
mail settings
Submitter Richard Sandiford
Date Jan. 19, 2014, 5:33 p.m.
Message ID <87lhyblvv8.fsf@talisman.default>
Download mbox | patch
Permalink /patch/312386/
State New
Headers show

Comments

Richard Sandiford - Jan. 19, 2014, 5:33 p.m.
The MIPS %hi and %lo relocation operators act as a pair on REL targets;
you need to see the partnering %lo in order to calculate the addend for
a %hi and detect carries properly.  The assembler therefore complains if
%hi is used without a corresponding %lo.

The MIPS backend tries to remove unmatched %his in mips_reorg.
The problem is that it can wrongly keep a %hi(foo) in cases like:

   asm ("" : "=m" (foo));

where a LO_SUM MEM appears as an operand at the rtl level but where
%lo(foo) doesn't appear in the assembly code.

It's very hard without fully parsing an asm to know whether it will need
the LO_SUM or not.  E.g. the operand could appear in the asm string
but could be protected by an .if or only be used in a comment.  But it
should be safe to assume that the LO_SUM in operand N isn't needed if
%N or %<letter>N doesn't appear in the string, since AFAIK there's
no other way for the asm to know which register contains the high part.
Specifically, things like:

   asm ("sw $0,foo" : "=m" (foo));

work fine either way, since the %hi is implicit in the SW macro.

I split out the routine to detect which operands are used since it
seemed like it might be more generally useful.

Tested on mips64-linux-gnu, where it fixes the gcc.dg/guality/pr36728-2.c
failures for -mabi=32.  OK for the target-independent bits?

Thanks,
Richard


gcc/
	PR target/52125
	* rtl.h (get_referenced_operands): Declare.
	* recog.c (get_referenced_operands): New function.
	* config/mips/mips.c (mips_reorg_process_insns): Check which asm
	operands have been referenced when recording LO_SUM references.

gcc/testsuite/
	PR target/52125
	* gcc.dg/pr48774.c: Remove skip for mips_rel.
	* gcc.target/mips/pr52125.c: New test.
Jeff Law - Jan. 23, 2014, 4:12 a.m.
On 01/19/14 10:33, Richard Sandiford wrote:
> The MIPS %hi and %lo relocation operators act as a pair on REL targets;
> you need to see the partnering %lo in order to calculate the addend for
> a %hi and detect carries properly.  The assembler therefore complains if
> %hi is used without a corresponding %lo.
>
> The MIPS backend tries to remove unmatched %his in mips_reorg.
> The problem is that it can wrongly keep a %hi(foo) in cases like:
>
>     asm ("" : "=m" (foo));
>
> where a LO_SUM MEM appears as an operand at the rtl level but where
> %lo(foo) doesn't appear in the assembly code.
>
> It's very hard without fully parsing an asm to know whether it will need
> the LO_SUM or not.  E.g. the operand could appear in the asm string
> but could be protected by an .if or only be used in a comment.  But it
> should be safe to assume that the LO_SUM in operand N isn't needed if
> %N or %<letter>N doesn't appear in the string, since AFAIK there's
> no other way for the asm to know which register contains the high part.
> Specifically, things like:
>
>     asm ("sw $0,foo" : "=m" (foo));
>
> work fine either way, since the %hi is implicit in the SW macro.
>
> I split out the routine to detect which operands are used since it
> seemed like it might be more generally useful.
>
> Tested on mips64-linux-gnu, where it fixes the gcc.dg/guality/pr36728-2.c
> failures for -mabi=32.  OK for the target-independent bits?
>
> Thanks,
> Richard
>
>
> gcc/
> 	PR target/52125
> 	* rtl.h (get_referenced_operands): Declare.
> 	* recog.c (get_referenced_operands): New function.
> 	* config/mips/mips.c (mips_reorg_process_insns): Check which asm
> 	operands have been referenced when recording LO_SUM references.
>
> gcc/testsuite/
> 	PR target/52125
> 	* gcc.dg/pr48774.c: Remove skip for mips_rel.
> 	* gcc.target/mips/pr52125.c: New test.
Machine independent bits are fine with me.

Jeff

Patch

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-01-18 09:57:11.042577719 +0000
+++ gcc/rtl.h	2014-01-19 11:01:36.970759960 +0000
@@ -2169,6 +2169,7 @@  extern rtx extract_asm_operands (rtx);
 extern int asm_noperands (const_rtx);
 extern const char *decode_asm_operands (rtx, rtx *, rtx **, const char **,
 					enum machine_mode *, location_t *);
+extern void get_referenced_operands (const char *, bool *, unsigned int);
 
 extern enum reg_class reg_preferred_class (int);
 extern enum reg_class reg_alternate_class (int);
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-01-02 22:16:05.816301145 +0000
+++ gcc/recog.c	2014-01-19 11:01:00.097426240 +0000
@@ -1620,6 +1620,50 @@  decode_asm_operands (rtx body, rtx *oper
   return ASM_OPERANDS_TEMPLATE (asmop);
 }
 
+/* Parse inline assembly string STRING and determine which operands are
+   referenced by % markers.  For the first NOPERANDS operands, set USED[I]
+   to true if operand I is referenced.
+
+   This is intended to distinguish barrier-like asms such as:
+
+      asm ("" : "=m" (...));
+
+   from real references such as:
+
+      asm ("sw\t$0, %0" : "=m" (...));  */
+
+void
+get_referenced_operands (const char *string, bool *used,
+			 unsigned int noperands)
+{
+  memset (used, 0, sizeof (bool) * noperands);
+  const char *p = string;
+  while (*p)
+    switch (*p)
+      {
+      case '%':
+	p += 1;
+	/* A letter followed by a digit indicates an operand number.  */
+	if (ISALPHA (p[0]) && ISDIGIT (p[1]))
+	  p += 1;
+	if (ISDIGIT (*p))
+	  {
+	    char *endptr;
+	    unsigned long opnum = strtoul (p, &endptr, 10);
+	    if (endptr != p && opnum < noperands)
+	      used[opnum] = true;
+	    p = endptr;
+	  }
+	else
+	  p += 1;
+	break;
+
+      default:
+	p++;
+	break;
+      }
+}
+
 /* Check if an asm_operand matches its constraints.
    Return > 0 if ok, = 0 if bad, < 0 if inconclusive.  */
 
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2014-01-18 09:57:11.628582243 +0000
+++ gcc/config/mips/mips.c	2014-01-19 11:31:18.743731497 +0000
@@ -16095,7 +16095,23 @@  mips_reorg_process_insns (void)
   for (insn = get_insns (); insn != 0; insn = NEXT_INSN (insn))
     FOR_EACH_SUBINSN (subinsn, insn)
       if (USEFUL_INSN_P (subinsn))
-	for_each_rtx (&PATTERN (subinsn), mips_record_lo_sum, &htab);
+	{
+	  rtx body = PATTERN (insn);
+	  int noperands = asm_noperands (body);
+	  if (noperands >= 0)
+	    {
+	      rtx *ops = XALLOCAVEC (rtx, noperands);
+	      bool *used = XALLOCAVEC (bool, noperands);
+	      const char *string = decode_asm_operands (body, ops, NULL, NULL,
+							NULL, NULL);
+	      get_referenced_operands (string, used, noperands);
+	      for (int i = 0; i < noperands; ++i)
+		if (used[i])
+		  for_each_rtx (&ops[i], mips_record_lo_sum, &htab);
+	    }
+	  else
+	    for_each_rtx (&PATTERN (subinsn), mips_record_lo_sum, &htab);
+	}
 
   last_insn = 0;
   hilo_delay = 2;
Index: gcc/testsuite/gcc.dg/pr48774.c
===================================================================
--- gcc/testsuite/gcc.dg/pr48774.c	2012-02-05 14:56:30.000000000 +0000
+++ gcc/testsuite/gcc.dg/pr48774.c	2014-01-19 11:38:10.059263519 +0000
@@ -1,6 +1,5 @@ 
 /* PR target/48774 */
 /* { dg-do run } */
-/* { dg-skip-if "PR 52125" { mips_rel } { "*" } { "" } } */
 /* { dg-options "-O2 -funroll-loops" } */
 
 extern void abort (void);
Index: gcc/testsuite/gcc.target/mips/pr52125.c
===================================================================
--- /dev/null	2013-12-26 20:29:50.272541227 +0000
+++ gcc/testsuite/gcc.target/mips/pr52125.c	2014-01-19 11:32:07.305148499 +0000
@@ -0,0 +1,20 @@ 
+/* { dg-options "addressing=absolute" } */
+
+int a, b, c, d;
+
+NOMIPS16 void
+foo (void)
+{
+  asm ("%1 %z3"
+       : "=m" (a), "=m" (b)
+       : "m" (c), "m" (d));
+}
+
+/* { dg-final { scan-assembler-not "%hi\\(a\\)" } } */
+/* { dg-final { scan-assembler-not "%lo\\(a\\)" } } */
+/* { dg-final { scan-assembler "%hi\\(b\\)" } } */
+/* { dg-final { scan-assembler "%lo\\(b\\)" } } */
+/* { dg-final { scan-assembler-not "%hi\\(c\\)" } } */
+/* { dg-final { scan-assembler-not "%lo\\(c\\)" } } */
+/* { dg-final { scan-assembler "%hi\\(d\\)" } } */
+/* { dg-final { scan-assembler "%lo\\(d\\)" } } */