* [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code
@ 2019-06-03 19:07 Faraz Shahbazker
2019-06-04 21:07 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Faraz Shahbazker @ 2019-06-03 19:07 UTC (permalink / raw)
To: gcc-patches; +Cc: Faraz Shahbazker, Jeff Law, Matthew Fortune
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 <macro@linux-mips.org>
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 (&mips_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 <u>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 0000000..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_<X>, @function # Symbol # must match label.
+__pend_foo_<X>: # The symbol must match.
+ .insn
+.L<Y>:
+
+2. No __pend symbol is emitted for back-to-back pools, like
+
+__pend_foo_<X>:
+ .insn
+ .type __pool_foo_<X>, @object
+
+3. No __pend symbol is emitted at the end of a function, like
+
+__pend_foo_<X>:
+ .insn
+ .end foo
+
+ */
+
+/* { 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 `-mplt' to prevent the special `__gnu_local_gp'
symbol from being placed in the constant pool at `-O0' for SVR4 code
and consequently interfering with test expectations. */
-/* { dg-final { scan-assembler "\tl\[wd\]\t\\\$\[0-9\]+,(.L(\[0-9\]+))\n.*\t\\.type\t(__pool_frob_\\2), @object\n\\3:\n\t\\.align\t2\n\\1:\n\t\\.d?word\t305419896\n\t\\.type\t(__pend_frob_\\2), @function\n\\4:\n\t\\.insn\n" } } */
+/* { dg-final { scan-assembler "\tlw\t\\\$\[0-9\]+,(.L(\[0-9\]+))\n.*\t\\.type\t(__pool_frob_\\2), @object\n\\3:\n\t\\.align\t2\n\\1:\n\t\\.word\t305419896\n" } } */
--
2.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code
2019-06-03 19:07 [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code Faraz Shahbazker
@ 2019-06-04 21:07 ` Maciej W. Rozycki
2019-06-05 22:34 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2019-06-04 21:07 UTC (permalink / raw)
To: Faraz Shahbazker; +Cc: gcc-patches, Jeff Law, Matthew Fortune
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code
2019-06-04 21:07 ` Maciej W. Rozycki
@ 2019-06-05 22:34 ` Jeff Law
2019-06-05 22:48 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2019-06-05 22:34 UTC (permalink / raw)
To: Maciej W. Rozycki, Faraz Shahbazker; +Cc: gcc-patches, Matthew Fortune
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code
2019-06-05 22:34 ` Jeff Law
@ 2019-06-05 22:48 ` Maciej W. Rozycki
2019-06-08 21:21 ` [PATCH v2] " Faraz Shahbazker
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2019-06-05 22:48 UTC (permalink / raw)
To: Jeff Law; +Cc: Faraz Shahbazker, gcc-patches, Matthew Fortune
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] [MIPS] Inhibit trailing .insn if pool is not followed by code
2019-06-05 22:48 ` Maciej W. Rozycki
@ 2019-06-08 21:21 ` Faraz Shahbazker
2019-06-11 20:10 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Faraz Shahbazker @ 2019-06-08 21:21 UTC (permalink / raw)
To: gcc-patches; +Cc: Maciej W . Rozycki, Matthew Fortune, Jeff Law
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. Change the
type of the __pend symbol depending on whether it is followed by
further code to inhibit the trailing .insn.
Based on original patch from Maciej W. Rozycki <macro@linux-mips.org>
gcc/
* config/mips/mips.c (mips_final_postscan_insn): Modify call
to `mips_set_text_contents_type' to indicate whether a
non-debug insn follows.
gcc/gcc/testsuite/
* gcc.target/mips/data-sym-pool.c: Update expected output.
* gcc.target/mips/data-sym-multi-pool.c: New test.
---
gcc/config/mips/mips.c | 16 ++++++--
.../gcc.target/mips/data-sym-multi-pool.c | 45 ++++++++++++++++++++++
gcc/testsuite/gcc.target/mips/data-sym-pool.c | 5 +--
3 files changed, 60 insertions(+), 6 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..0e1a68a 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -20636,9 +20636,19 @@ mips_final_postscan_insn (FILE *file ATTRIBUTE_UNUSED, rtx_insn *insn,
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);
+ {
+ rtx_insn *next_insn = next_real_nondebug_insn (insn);
+ bool code_p = (next_insn != NULL
+ && INSN_P (next_insn)
+ && (GET_CODE (PATTERN (next_insn)) != UNSPEC_VOLATILE
+ || XINT (PATTERN (next_insn), 1) != UNSPEC_CONSTTABLE));
+
+ /* Switch content type depending on whether there is code beyond
+ the constant pool. */
+ mips_set_text_contents_type (asm_out_file, "__pend_",
+ INTVAL (XVECEXP (PATTERN (insn), 0, 0)),
+ code_p);
+ }
}
/* Return the function that is used to expand the <u>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 0000000..1936f5b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/data-sym-multi-pool.c
@@ -0,0 +1,45 @@
+/* { 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 is emitted as STT_FUNCTION followed by instructions:
+ .type __pend_frob_<X>, @function # Symbol # must match label.
+__pend_foo_<X>: # The symbol must match.
+ .insn
+.L<Y>:
+
+2. __pend symbol at end of function has type STT_OBJECT
+
+ .type __pend_foo_<X>, @object
+__pend_foo_<X>:
+ .insn
+ .end foo
+
+ */
+
+/* { 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 "\t\\.type\t(__pend_foo_\[0-9\]+), @object\n\\1:\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..f093511 100644
--- a/gcc/testsuite/gcc.target/mips/data-sym-pool.c
+++ b/gcc/testsuite/gcc.target/mips/data-sym-pool.c
@@ -16,9 +16,8 @@ __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.
+ .type __pend_frob_3, @object # Symbol # must match label.
__pend_frob_3: # The symbol must match.
- .insn
that is `__pool_*'/`__pend_*' symbols inserted around a constant pool.
@@ -26,4 +25,4 @@ __pend_frob_3: # The symbol must match.
symbol from being placed in the constant pool at `-O0' for SVR4 code
and consequently interfering with test expectations. */
-/* { dg-final { scan-assembler "\tl\[wd\]\t\\\$\[0-9\]+,(.L(\[0-9\]+))\n.*\t\\.type\t(__pool_frob_\\2), @object\n\\3:\n\t\\.align\t2\n\\1:\n\t\\.d?word\t305419896\n\t\\.type\t(__pend_frob_\\2), @function\n\\4:\n\t\\.insn\n" } } */
+/* { dg-final { scan-assembler "\tl\[wd\]\t\\\$\[0-9\]+,(.L(\[0-9\]+))\n.*\t\\.type\t(__pool_frob_\\2), @object\n\\3:\n\t\\.align\t2\n\\1:\n\t\\.d?word\t305419896\n\t\\.type\t(__pend_frob_\\2), @object\n\\4:\n" } } */
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] [MIPS] Inhibit trailing .insn if pool is not followed by code
2019-06-08 21:21 ` [PATCH v2] " Faraz Shahbazker
@ 2019-06-11 20:10 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2019-06-11 20:10 UTC (permalink / raw)
To: Faraz Shahbazker, gcc-patches; +Cc: Maciej W . Rozycki, Matthew Fortune
On 6/8/19 3:21 PM, 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. Change the
> type of the __pend symbol depending on whether it is followed by
> further code to inhibit the trailing .insn.
>
> Based on original patch from Maciej W. Rozycki <macro@linux-mips.org>
>
> gcc/
> * config/mips/mips.c (mips_final_postscan_insn): Modify call
> to `mips_set_text_contents_type' to indicate whether a
> non-debug insn follows.
>
> gcc/gcc/testsuite/
> * gcc.target/mips/data-sym-pool.c: Update expected output.
> * gcc.target/mips/data-sym-multi-pool.c: New test.
THanks. I've installed this on the trunk.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-11 20:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 19:07 [PATCH] [MIPS] Inhibit trailing .insn if pool is not followed by code Faraz Shahbazker
2019-06-04 21:07 ` Maciej W. Rozycki
2019-06-05 22:34 ` Jeff Law
2019-06-05 22:48 ` Maciej W. Rozycki
2019-06-08 21:21 ` [PATCH v2] " Faraz Shahbazker
2019-06-11 20:10 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).