Fix sim fallout from arm assembler complaining about symbols named as insns

Fix sim fallout from arm assembler complaining about symbols named as insns

[Hans-Peter Nilsson]

See commit 8b2d793ce5e and
<https://sourceware.org/bugzilla/show_bug.cgi?id=18347>.
I'm not completely sure this new gas warning is a good thing.
I mean, symbols such as those below don't really interfere with
the insn namespace, do they?  Though, I'm somewhat sure that
plain silencing the warning by adding (if supported) the new gas
option -mno-warn-sym to the arm sim test-suite would be slightly
uglier than the following; renaming the symbols.  Either way,
Nick (or anyone else), if prefer something else than what's
below, don't let me get in the way of you fixing it to your
liking.

To wit, right now, the new symbol "sanity-check" causes failures
for --target arm-eabi check-sim:

Running /tmp/hpautotest-sim/src/sim/testsuite/sim/arm/allinsn.exp ...
FAIL: xscale bl.cgs (assembling)
Running /tmp/hpautotest-sim/src/sim/testsuite/sim/arm/iwmmxt/iwmmxt.exp ...
FAIL: xscale tmia.cgs (assembling)
FAIL: xscale tmiaph.cgs (assembling)
FAIL: xscale waligni.cgs (assembling)
FAIL: xscale wand.cgs (assembling)
FAIL: xscale wandn.cgs (assembling)
FAIL: xscale wmov.cgs (assembling)
FAIL: xscale wor.cgs (assembling)
FAIL: xscale wshufh.cgs (assembling)
FAIL: xscale wxor.cgs (assembling)
FAIL: xscale wzero.cgs (assembling)
Running /tmp/hpautotest-sim/src/sim/testsuite/sim/arm/misc.exp ...
Running /tmp/hpautotest-sim/src/sim/testsuite/sim/arm/thumb/allthumb.exp ...
Running /tmp/hpautotest-sim/src/sim/testsuite/sim/arm/xscale/xscale.exp ...
FAIL: xscale mia.cgs (assembling)
FAIL: xscale miaph.cgs (assembling)

sim.log:
x/src/sim/testsuite/sim/arm/bl.cgs: Assembler messages:
x/src/sim/testsuite/sim/arm/bl.cgs:8: Warning: [-mwarn-syms]: Symbol 'bl' matches an ARM instruction - is this intentional ?

FAIL: xscale bl.cgs (assembling)

(etc.)

My suggestion follows.  I'm holding off for a day or three.

diff --git a/sim/testsuite/sim/arm/ChangeLog b/sim/testsuite/sim/arm/ChangeLog
index 1237d81..83dbd79 100644
--- a/sim/testsuite/sim/arm/ChangeLog
+++ b/sim/testsuite/sim/arm/ChangeLog
@@ -1,3 +1,19 @@
+2015-05-02  Hans-Peter Nilsson  <hp@axis.com>
+
+	* bl.cgs (bl0): Rename from symbol colliding with insn name bl.
+	* iwmmxt/tmia.cgs (tmia0): Similar.
+	* iwmmxt/tmiaph.cgs (tmiaph0): Similar.
+	* iwmmxt/waligni.cgs (waligni0): Similar.
+	* iwmmxt/wand.cgs (wand0): Similar.
+	* iwmmxt/wandn.cgs (wandn0): Similar.
+	* iwmmxt/wmov.cgs (wmov0): Similar.
+	* iwmmxt/wor.cgs (wor0): Similar.
+	* iwmmxt/wshufh.cgs (wshuf0): Similar.
+	* iwmmxt/wxor.cgs (wxor0): Similar.
+	* iwmmxt/wzero.cgs (wzero0): Similar.
+	* xscale/mia.cgs (mia0): Similar.
+	* xscale/miaph.cgs (miaph0): Similar.
+
 2013-05-07  Jayant Sonar  <jayant.sonar@kpitcummins.com>
 	    Kaushik Phatak <Kaushik.Phatak@kpitcummins.com>
 
diff --git a/sim/testsuite/sim/arm/bl.cgs b/sim/testsuite/sim/arm/bl.cgs
index fbc7ef5..2655d3d 100644
--- a/sim/testsuite/sim/arm/bl.cgs
+++ b/sim/testsuite/sim/arm/bl.cgs
@@ -5,8 +5,8 @@
 
 	start
 
-	.global bl
-bl:
+	.global bl0
+bl0:
 	mvi_h_gr r14,0
 	bl bl2
 bl1:
diff --git a/sim/testsuite/sim/arm/iwmmxt/tmia.cgs b/sim/testsuite/sim/arm/iwmmxt/tmia.cgs
index 0b0da66..8c6e0dd 100644
--- a/sim/testsuite/sim/arm/iwmmxt/tmia.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/tmia.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global tmia
-tmia:
+	.global tmia0
+tmia0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/tmiaph.cgs b/sim/testsuite/sim/arm/iwmmxt/tmiaph.cgs
index 3778b0a..820065a 100644
--- a/sim/testsuite/sim/arm/iwmmxt/tmiaph.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/tmiaph.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global tmiaph
-tmiaph:
+	.global tmiaph0
+tmiaph0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/waligni.cgs b/sim/testsuite/sim/arm/iwmmxt/waligni.cgs
index dc99dae..174e889 100644
--- a/sim/testsuite/sim/arm/iwmmxt/waligni.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/waligni.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global waligni
-waligni:
+	.global waligni0
+waligni0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wand.cgs b/sim/testsuite/sim/arm/iwmmxt/wand.cgs
index 018383f..6ac45da 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wand.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wand.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wand
-wand:
+	.global wand0
+wand0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wandn.cgs b/sim/testsuite/sim/arm/iwmmxt/wandn.cgs
index f2c2305..907e3d0 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wandn.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wandn.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wandn
-wandn:
+	.global wandn0
+wandn0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wmov.cgs b/sim/testsuite/sim/arm/iwmmxt/wmov.cgs
index e86fed6..b7f88d1 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wmov.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wmov.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wmov
-wmov:
+	.global wmov0
+wmov0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wor.cgs b/sim/testsuite/sim/arm/iwmmxt/wor.cgs
index 48d5f53..6950beb 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wor.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wor.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wor
-wor:
+	.global wor0
+wor0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wshufh.cgs b/sim/testsuite/sim/arm/iwmmxt/wshufh.cgs
index d5cff1e..15b15bc 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wshufh.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wshufh.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wshufh
-wshufh:
+	.global wshufh0
+wshufh0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wxor.cgs b/sim/testsuite/sim/arm/iwmmxt/wxor.cgs
index 95e1fc8..922a30f 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wxor.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wxor.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wxor
-wxor:
+	.global wxor0
+wxor0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/iwmmxt/wzero.cgs b/sim/testsuite/sim/arm/iwmmxt/wzero.cgs
index 78fa7c5..52df3c8 100644
--- a/sim/testsuite/sim/arm/iwmmxt/wzero.cgs
+++ b/sim/testsuite/sim/arm/iwmmxt/wzero.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global wzero
-wzero:
+	.global wzero0
+wzero0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/xscale/mia.cgs b/sim/testsuite/sim/arm/xscale/mia.cgs
index a3f729e..cfdc160 100644
--- a/sim/testsuite/sim/arm/xscale/mia.cgs
+++ b/sim/testsuite/sim/arm/xscale/mia.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global mia
-mia:
+	.global mia0
+mia0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 
diff --git a/sim/testsuite/sim/arm/xscale/miaph.cgs b/sim/testsuite/sim/arm/xscale/miaph.cgs
index 53fb201..c97b858 100644
--- a/sim/testsuite/sim/arm/xscale/miaph.cgs
+++ b/sim/testsuite/sim/arm/xscale/miaph.cgs
@@ -6,8 +6,8 @@
 
 	start
 
-	.global miaph
-miaph:
+	.global miaph0
+miaph0:
 	# Enable access to CoProcessors 0 & 1 before
         # we attempt these instructions.
 

brgds, H-P

Re: Fix sim fallout from arm assembler complaining about symbols named as insns

[Nicholas Clifton]

Hi Hans-Peter,

> I'm not completely sure this new gas warning is a good thing.
> I mean, symbols such as those below don't really interfere with
> the insn namespace, do they?

No, but they can be a little bit confusing and the problem I was trying 
to solve, of an instruction name being mistakenly treated as a symbol, 
is genuine. It would be better I agree to restrict this check to just 
the case where the "=" assignment operator is being used, but I did not 
want to modify generic code.  Maybe I should have done that. :-(


> To wit, right now, the new symbol "sanity-check" causes failures
> for --target arm-eabi check-sim:

So it does.  I should have checked that before committing the patch.  Sorry.


> +2015-05-02  Hans-Peter Nilsson  <hp@axis.com>
> +
> +	* bl.cgs (bl0): Rename from symbol colliding with insn name bl.
> +	* iwmmxt/tmia.cgs (tmia0): Similar.
> +	* iwmmxt/tmiaph.cgs (tmiaph0): Similar.
> +	* iwmmxt/waligni.cgs (waligni0): Similar.
> +	* iwmmxt/wand.cgs (wand0): Similar.
> +	* iwmmxt/wandn.cgs (wandn0): Similar.
> +	* iwmmxt/wmov.cgs (wmov0): Similar.
> +	* iwmmxt/wor.cgs (wor0): Similar.
> +	* iwmmxt/wshufh.cgs (wshuf0): Similar.
> +	* iwmmxt/wxor.cgs (wxor0): Similar.
> +	* iwmmxt/wzero.cgs (wzero0): Similar.
> +	* xscale/mia.cgs (mia0): Similar.
> +	* xscale/miaph.cgs (miaph0): Similar.

I think that this is a good solution - please apply.

Cheers
   Nick

Re: Fix sim fallout from arm assembler complaining about symbols named as insns

[Vidya Praveen]

On Wed, May 06, 2015 at 11:37:16AM +0100, Nicholas Clifton wrote:
> Hi Hans-Peter,
> 
> > I'm not completely sure this new gas warning is a good thing.
> > I mean, symbols such as those below don't really interfere with
> > the insn namespace, do they?
> 
> No, but they can be a little bit confusing and the problem I was trying 
> to solve, of an instruction name being mistakenly treated as a symbol, 
> is genuine. It would be better I agree to restrict this check to just 
> the case where the "=" assignment operator is being used, but I did not 
> want to modify generic code.  Maybe I should have done that. :-(

Yes. I think the warning should just restrict to cases where it can possibly
go wrong. There can be too many false positives with the current solution as
it warns if a symbol name merely matches a mnemonic. 

For example, simply having a global variable called "str" in a C program (which
is perfectly valid) can trigger this warning. In fact quite a few gcc tests 
fail at the moment because of this. 

VP.




> > To wit, right now, the new symbol "sanity-check" causes failures
> > for --target arm-eabi check-sim:
> 
> So it does.  I should have checked that before committing the patch.  Sorry.
> 
> 
> > +2015-05-02  Hans-Peter Nilsson  <hp@axis.com>
> > +
> > +	* bl.cgs (bl0): Rename from symbol colliding with insn name bl.
> > +	* iwmmxt/tmia.cgs (tmia0): Similar.
> > +	* iwmmxt/tmiaph.cgs (tmiaph0): Similar.
> > +	* iwmmxt/waligni.cgs (waligni0): Similar.
> > +	* iwmmxt/wand.cgs (wand0): Similar.
> > +	* iwmmxt/wandn.cgs (wandn0): Similar.
> > +	* iwmmxt/wmov.cgs (wmov0): Similar.
> > +	* iwmmxt/wor.cgs (wor0): Similar.
> > +	* iwmmxt/wshufh.cgs (wshuf0): Similar.
> > +	* iwmmxt/wxor.cgs (wxor0): Similar.
> > +	* iwmmxt/wzero.cgs (wzero0): Similar.
> > +	* xscale/mia.cgs (mia0): Similar.
> > +	* xscale/miaph.cgs (miaph0): Similar.
> 
> I think that this is a good solution - please apply.

 

Re: Fix sim fallout from arm assembler complaining about symbols named as insns

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

On 06 May 2015 11:37, Nicholas Clifton wrote:
> Hi Hans-Peter,
> 
> > I'm not completely sure this new gas warning is a good thing.
> > I mean, symbols such as those below don't really interfere with
> > the insn namespace, do they?
> 
> No, but they can be a little bit confusing and the problem I was trying 
> to solve, of an instruction name being mistakenly treated as a symbol, 
> is genuine. It would be better I agree to restrict this check to just 
> the case where the "=" assignment operator is being used, but I did not 
> want to modify generic code.  Maybe I should have done that. :-(
> 
> 
> > To wit, right now, the new symbol "sanity-check" causes failures
> > for --target arm-eabi check-sim:
> 
> So it does.  I should have checked that before committing the patch.  Sorry.
> 
> 
> > +2015-05-02  Hans-Peter Nilsson  <hp@axis.com>
> > +
> > +	* bl.cgs (bl0): Rename from symbol colliding with insn name bl.
> > +	* iwmmxt/tmia.cgs (tmia0): Similar.
> > +	* iwmmxt/tmiaph.cgs (tmiaph0): Similar.
> > +	* iwmmxt/waligni.cgs (waligni0): Similar.
> > +	* iwmmxt/wand.cgs (wand0): Similar.
> > +	* iwmmxt/wandn.cgs (wandn0): Similar.
> > +	* iwmmxt/wmov.cgs (wmov0): Similar.
> > +	* iwmmxt/wor.cgs (wor0): Similar.
> > +	* iwmmxt/wshufh.cgs (wshuf0): Similar.
> > +	* iwmmxt/wxor.cgs (wxor0): Similar.
> > +	* iwmmxt/wzero.cgs (wzero0): Similar.
> > +	* xscale/mia.cgs (mia0): Similar.
> > +	* xscale/miaph.cgs (miaph0): Similar.
> 
> I think that this is a good solution - please apply.

so people can't have a global variable named "bl" now ?  or "ldr" ?  that 
doesn't seem like the right direction in which case this patch isn't really 
needed ...
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Fix sim fallout from arm assembler complaining about symbols named as insns

[Hans-Peter Nilsson]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Thu, 7 May 2015 08:36:14 +0200

> so people can't have a global variable named "bl" now ?  or "ldr" ?  that 
> doesn't seem like the right direction in which case this patch isn't really 
> needed ...
> -mike

Yeah, as you've noticed I haven't applied it yet.

I reconsidered after VP noting that there actually *are* gcc
test-suite cases failing because of this.

Sorry Nick, but I'd like to see a -mwarn-syms being passed by
default by gcc, before believing this warning-by-default
behavior is here to stay.  The cure (helping a rare programming
error with an unexpected failure mode by means of a sledgehammer
fix) is IMHO worse than the disease.

Add a target hook to the SYM = EXPR gas code and check there?
(But don't forget to check that the symbol doesn't yet exist or
at least isn't global.)

brgds, H-P