Re: [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code

2019-06-05 Thread Maciej W. Rozycki
On Wed, 5 Jun 2019, Jeff Law wrote:

> >  Thanks for looking into it.  FWIW I think the `__pend' symbol will best 
> > be still emitted for consistency, however as STT_OBJECT and consequently 
> > with no trailing `.insn'.
> If I understand correctly we'd still want to call
> mips_set_text_contents_type in all the cases we did before, but that
> we'd pass in false for the FUNCTION_P argument?

 Yes, I think it would be the most straightforward implementation.

  Maciej


Re: [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code

2019-06-05 Thread Jeff Law
On 6/4/19 3:07 PM, Maciej W. Rozycki wrote:
> On Mon, 3 Jun 2019, Faraz Shahbazker wrote:
> 
>> The __pool and __pend symbols are used to mark the beginning and end of
>> inline constant pools in MIPS16 code regions.  However if the pool occurs
>> at the boundary of a code region and is not followed by further code,
>> presence of the __pend symbol can confuse the dissassembler in to treating
>> subsequent non-MIPS16 code block as MIPS16.
> 
>  Thanks for looking into it.  FWIW I think the `__pend' symbol will best 
> be still emitted for consistency, however as STT_OBJECT and consequently 
> with no trailing `.insn'.
If I understand correctly we'd still want to call
mips_set_text_contents_type in all the cases we did before, but that
we'd pass in false for the FUNCTION_P argument?

jeff


Re: [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code

2019-06-04 Thread Maciej W. Rozycki
On Mon, 3 Jun 2019, Faraz Shahbazker wrote:

> The __pool and __pend symbols are used to mark the beginning and end of
> inline constant pools in MIPS16 code regions.  However if the pool occurs
> at the boundary of a code region and is not followed by further code,
> presence of the __pend symbol can confuse the dissassembler in to treating
> subsequent non-MIPS16 code block as MIPS16.

 Thanks for looking into it.  FWIW I think the `__pend' symbol will best 
be still emitted for consistency, however as STT_OBJECT and consequently 
with no trailing `.insn'.

  Maciej


[PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code

2019-06-03 Thread Faraz Shahbazker
The __pool and __pend symbols are used to mark the beginning and end of
inline constant pools in MIPS16 code regions.  However if the pool occurs
at the boundary of a code region and is not followed by further code,
presence of the __pend symbol can confuse the dissassembler in to treating
subsequent non-MIPS16 code block as MIPS16.

Based on original patch from Maciej W. Rozycki 

gcc/
* config/mips/mips.c (mips_final_postscan_insn): Only call
`mips_set_text_contents_type' if a non-debug insn follows.

gcc/gcc/testsuite/
* gcc.target/mips/data-sym-pool.c: Don't check for
__pend symbol.
* gcc.target/mips/data-sym-multi-pool.c: New test.
---
 gcc/config/mips/mips.c | 18 ++--
 .../gcc.target/mips/data-sym-multi-pool.c  | 51 ++
 gcc/testsuite/gcc.target/mips/data-sym-pool.c  |  7 +--
 3 files changed, 67 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/mips/data-sym-multi-pool.c

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index c6433dc..3fb6c9d 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -20630,15 +20630,25 @@ static void
 mips_final_postscan_insn (FILE *file ATTRIBUTE_UNUSED, rtx_insn *insn,
  rtx *opvec, int noperands)
 {
+  rtx_insn *next_insn;
+
   if (mips_need_noat_wrapper_p (insn, opvec, noperands))
 mips_pop_asm_switch (_noat);
 
   if (INSN_P (insn)
   && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
-  && XINT (PATTERN (insn), 1) == UNSPEC_CONSTTABLE_END)
-mips_set_text_contents_type (asm_out_file, "__pend_",
-INTVAL (XVECEXP (PATTERN (insn), 0, 0)),
-TRUE);
+  && XINT (PATTERN (insn), 1) == UNSPEC_CONSTTABLE_END
+  && (next_insn = next_real_nondebug_insn (insn)))
+{
+  /* Switch content type only if we know there is code beyond
+the constant pool.  */
+  if (INSN_P (next_insn)
+ && (GET_CODE (PATTERN (next_insn)) != UNSPEC_VOLATILE
+ || XINT (PATTERN (next_insn), 1) != UNSPEC_CONSTTABLE))
+   mips_set_text_contents_type (asm_out_file, "__pend_",
+INTVAL (XVECEXP (PATTERN (insn), 0, 0)),
+TRUE);
+}
 }
 
 /* Return the function that is used to expand the mulsidi3 pattern.
diff --git a/gcc/testsuite/gcc.target/mips/data-sym-multi-pool.c 
b/gcc/testsuite/gcc.target/mips/data-sym-multi-pool.c
new file mode 100644
index 000..2db90e3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/data-sym-multi-pool.c
@@ -0,0 +1,51 @@
+/* { dg-do compile } */
+/* { dg-options "-mips16 -mcode-readable=yes" } */
+/* { dg-skip-if "per-function expected output" { *-*-* } { "-flto" } { "" } } 
*/
+
+/* This testcase generates multiple constant pools within a function body.  */
+
+#define C(a,b) \
+  if (a > b)  goto gt; \
+  if (a < b)  goto lt;
+
+#define C4(x,b) C((x)[0], b) C((x)[1],b) C((x)[2],b) C((x)[3],b)
+#define C16(x,y) C4(x, (y)[0]) C4(x, (y)[1]) C4(x, (y)[2]) C4(x, (y)[3])
+
+#define C64(x,y) C16(x,y) C16(x+4,y) C16(x+8,y)
+#define C256(x,y) C64(x,y) C64(x,y+4) C64(x,y+8)
+
+unsigned foo(int x[64], int y[64])
+{
+  C256(x,y);
+
+  return 0x01234567;
+ gt:
+  return 0x12345678;
+ lt:
+  return 0xF0123456;
+}
+
+/*  Check that:
+1. The __pend symbol, when emitted is followed by an instruction:
+   .type   __pend_frob_, @function  # Symbol # must match label.
+__pend_foo_:# The symbol must match.
+   .insn
+.L:
+
+2. No __pend symbol is emitted for back-to-back pools, like
+
+__pend_foo_:
+   .insn
+   .type   __pool_foo_, @object
+
+3. No __pend symbol is emitted at the end of a function, like
+
+__pend_foo_:
+   .insn
+   .endfoo
+
+  */
+
+/* { dg-final { scan-assembler "\t\\.type\t(__pend_foo_\[0-9\]+), 
@function\n\\1:\n\t\\.insn\n.L\[0-9\]+:\n" } }  */
+/* { dg-final { scan-assembler-not 
"__pend_foo_\[0-9\]+:\n\t\\.insn\n\t\\.type\t__pool_foo_\[0-9\]+, @object\n" } 
}  */
+/* { dg-final { scan-assembler-not 
"__pend_foo_\[0-9\]+:\n\t\\.insn\n\t\\.end\tfoo\n" } }  */
diff --git a/gcc/testsuite/gcc.target/mips/data-sym-pool.c 
b/gcc/testsuite/gcc.target/mips/data-sym-pool.c
index 8776d2b..8eac101 100644
--- a/gcc/testsuite/gcc.target/mips/data-sym-pool.c
+++ b/gcc/testsuite/gcc.target/mips/data-sym-pool.c
@@ -16,14 +16,11 @@ __pool_frob_3:  # The 
symbol must match.
.align  2
 $L3:   # The label must match.
.word   305419896
-   .type   __pend_frob_3, @function# Symbol # must match label.
-__pend_frob_3: # The symbol must match.
-   .insn
 
-   that is `__pool_*'/`__pend_*' symbols inserted around a constant pool.
+   that is `__pool_*' symbol is inserted before a constant pool.
 
This code is built with