Fix sim fallout from arm assembler complaining about symbols named as insns
Commit Message
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.
brgds, H-P
Comments
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
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.
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
@@ -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>
@@ -5,8 +5,8 @@
start
- .global bl
-bl:
+ .global bl0
+bl0:
mvi_h_gr r14,0
bl bl2
bl1:
@@ -6,8 +6,8 @@
start
- .global tmia
-tmia:
+ .global tmia0
+tmia0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global tmiaph
-tmiaph:
+ .global tmiaph0
+tmiaph0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global waligni
-waligni:
+ .global waligni0
+waligni0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wand
-wand:
+ .global wand0
+wand0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wandn
-wandn:
+ .global wandn0
+wandn0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wmov
-wmov:
+ .global wmov0
+wmov0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wor
-wor:
+ .global wor0
+wor0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wshufh
-wshufh:
+ .global wshufh0
+wshufh0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wxor
-wxor:
+ .global wxor0
+wxor0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global wzero
-wzero:
+ .global wzero0
+wzero0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global mia
-mia:
+ .global mia0
+mia0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.
@@ -6,8 +6,8 @@
start
- .global miaph
-miaph:
+ .global miaph0
+miaph0:
# Enable access to CoProcessors 0 & 1 before
# we attempt these instructions.