diff mbox series

[1/2] MIPS/GCC/testsuite: Fix data-sym-pool.c for SVR4 model at -O0

Message ID alpine.DEB.2.00.1804111319190.1545@tp.orcam.me.uk
State Accepted
Headers show
Series MIPS/GCC/testsuite: Fixes for data-sym-pool.c | expand

Commit Message

Maciej W. Rozycki April 11, 2018, 1:52 p.m. UTC 
With GCC configurations using the SVR4 rather than the PLT dynamic 
executable model and the o32 ABI with the data-sym-pool.c test case code 
like below is produced:

	.file	1 "data-sym-pool.c"
	.section .mdebug.abi32
	.previous
	.nan	legacy
	.module	fp=xx
	.module	nooddspreg
	.abicalls
	.text
	.align	2
	.globl	frob
	.set	mips16
	.set	nomicromips
	.ent	frob
	.type	frob, @function
frob:
	.frame	$17,8,$31		# vars= 0, regs= 1/0, args= 0, gp= 0
	.mask	0x00020000,-4
	.fmask	0x00000000,0
	save	8,$17
	move	$17,$sp
	lw	$2,$L4
	move	$sp,$17
	restore	8,$17
	jr	$31
	.type	__pool_frob_3, @object
__pool_frob_3:
	.align	2
$L3:
	.word	__gnu_local_gp
$L4:
	.word	305419896
	.type	__pend_frob_3, @function
__pend_frob_3:
	.insn
	.end	frob
	.size	frob, .-frob
	.ident	"GCC: (GNU) 8.0.1 20180410 (experimental)"

causing a failure due to the unexpected `__gnu_local_gp' entry in the 
constant pool, even though there is nothing wrong with it as far as the 
annotation being examined is concerned.

Given that the SVR4 vs PLT code model consideration is irrelevant for 
this test case rather than rewriting the regular expression to match 
this variant of code just enforce the PLT model by using the `-mplt' 
option.  It is safe to use this option unconditionally as it is silently 
ignored with configurations that do not support this model, e.g. bare 
metal ELF.

	gcc/testsuite/
	* gcc.target/mips/data-sym-pool.c (dg-options): Add `-mplt'.
---
Hi,

 I have regression-tested this with the `mips-mti-linux-gnu' target and 
the o32, n32 and n64 ABIs.  The two latters are demoted to o64 by the test 
framework due to the lack of MIPS16 support for the hard-float variants of 
these ABIs and I don't have soft-float multilibs configured, so instead I 
have verified n32/soft-float and n64/soft-float variants by hand.  The 
latter revealed the need for 2/2.

 Finally I do not have a bare metal ELF configuration available for 
regression-testing right now, so I only verified that `-mplt' is silently 
ignored.  Code generated is expected to be the same as in the PLT mode.

 OK to apply?

  Maciej
---
 gcc/testsuite/gcc.target/mips/data-sym-pool.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

gcc-mips16-test-data-sym-pool-extra-entries.diff

Comments

Matthew Fortune April 18, 2018, 3:02 p.m. UTC | #1 
Maciej Rozycki <Maciej.Rozycki@mips.com> writes:
> Given that the SVR4 vs PLT code model consideration is irrelevant for
> this test case rather than rewriting the regular expression to match
> this variant of code just enforce the PLT model by using the `-mplt'
> option.  It is safe to use this option unconditionally as it is silently
> ignored with configurations that do not support this model, e.g. bare
> metal ELF.
> 
> 	gcc/testsuite/
> 	* gcc.target/mips/data-sym-pool.c (dg-options): Add `-mplt'.
> ---
> Hi,
> 
>  I have regression-tested this with the `mips-mti-linux-gnu' target and
> the o32, n32 and n64 ABIs.  The two latters are demoted to o64 by the
> test framework due to the lack of MIPS16 support for the hard-float
> variants of these ABIs and I don't have soft-float multilibs configured,
> so instead I have verified n32/soft-float and n64/soft-float variants by
> hand.  The latter revealed the need for 2/2.
> 
>  Finally I do not have a bare metal ELF configuration available for
> regression-testing right now, so I only verified that `-mplt' is
> silently ignored.  Code generated is expected to be the same as in the
> PLT mode.
> 
>  OK to apply?

Hi Maciej,

Sorry for skimming past this and being slow to respond. I agree with your
choice of forcing an option to stabilise the generated code it does tend
to future proof better than leaving options floating.

OK to apply but given the release date I've added RMs to approve for
trunk right now.

Thanks,
Matthew
Maciej W. Rozycki April 26, 2018, 9:10 p.m. UTC | #2 
On Wed, 18 Apr 2018, Matthew Fortune wrote:

> OK to apply but given the release date I've added RMs to approve for
> trunk right now.

 Applied to trunk, now that GCC 8 has branched.  Thanks for your review.

  Maciej
diff mbox series

Patch

Index: gcc/gcc/testsuite/gcc.target/mips/data-sym-pool.c
===================================================================
--- gcc.orig/gcc/testsuite/gcc.target/mips/data-sym-pool.c	2016-11-17 21:24:46.000000000 +0000
+++ gcc/gcc/testsuite/gcc.target/mips/data-sym-pool.c	2018-04-10 23:27:49.226719338 +0100
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-mips16 -mcode-readable=yes" } */
+/* { dg-options "-mips16 -mcode-readable=yes -mplt" } */
 
 int
 frob (void)
@@ -20,6 +20,10 @@  $L3:						# The label must match.
 __pend_frob_3:					# The symbol must match.
 	.insn
 
-   that is `__pool_*'/`__pend_*' symbols inserted around a constant pool.  */
+   that is `__pool_*'/`__pend_*' symbols inserted around 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 "\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\t\\.type\t(__pend_frob_\\2), @function\n\\4:\n\t\\.insn\n" } } */