[PATCH ARM] Fixing problem of 32bit multiplication instruction 'smull'

[PATCH ARM] Fixing problem of 32bit multiplication instruction 'smull'

[Jayant R. Sonar]

Hi,

While testing a 32bit x 32bit multiplication instruction 'smull' 
using GDB, I faced a problem for Cortex-A9 target.

Consider the following sample assembly code.
Compilation command I used: arm-eabi-gcc -march=armv7-a test.s
==============================================================
        .global main
main:
        movw    r2, 0x5432
        movt    r2, 0x7698
        movw    r3, 0x5678
        movt    r3, 0x1234
        smull   r1, r3, r3, r2
        bx      lr
==============================================================

The "smull" instruction syntax is: 
	 SMULL{S}<c> <RdLo>, <RdHi>, <Rn>, <Rm>.
Here, the signed 32bit operands from Rm and Rn are multiplied and the
64 bit result is stored in the register pair RdHi, RdLo.

The GDB debugger throws the following error on executing 'smull':
     sim: MULTIPLY64 - INVALID ARGUMENTS

This was because the "Multiply64()" routine from ARM simulator source
file 'armemu.c' checks for a condition:
	 nRdHi != nRm &&  nRdLo != nRm
 
However, as per the "ARM Architecture Reference Manual ARMv7-A and 
ARMv7-R edition, ARM DDI 0406C.b, ID072512", section "A8.8.189 SMULL",
this condition is required only if "ArchVersion() < 6". 

Therefore, in case of Cortex-A9 target this condition should not be 
tested. However, this is not happening resulting in condition failure
and ultimately instruction execution failure.

Following change in the "Multiply64()" makes this check conditional 
and resolves the problem.

--- gdb-a/sim/arm/armemu.c      2013-05-14 15:20:47.000000000 +0530
+++ gdb-b/sim/arm/armemu.c      2013-05-14 15:23:20.000000000 +0530
@@ -5077,8 +5077,9 @@ Multiply64 (ARMul_State * state, ARMword
       && nRs   != 15
       && nRm   != 15
       && nRdHi != nRdLo
-      && nRdHi != nRm
-      && nRdLo != nRm)
+      && (state->is_v6
+          || (   nRdHi != nRm
+              && nRdLo != nRm)))
     {
       /* Intermediate results.  */
       ARMword lo, mid1, mid2, hi;

It alters the condition check for targets ARMv6 and above only.
Can someone please review this change?

Regression tested successfully for gdb and simulator with cortex-a9 
target. 

To add more here, for Cortex-A9 target, without this change I was
getting 14302 unexpected failures during GCC regression testing. Most
of them were execution failures, failing with same error message as I
was getting with above mentioned sample code. With this change 
the unexpected failure count has got reduced to just 1363.

Regards
Jayant

RE: [PATCH ARM] Fixing problem of 32bit multiplication instruction 'smull'

[Jayant R. Sonar]

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

Hello Mike,

Thanks again for reviewing this patch as well.

>> could you make that into a test case ? :)

I really don't have a test case in C form as of now.
However, when I was doing the GCC regression testing, I found almost
in all of the execution tests, final object files are linked with
another object file 'gcc_tg.o'. Objdump of this file shows 'smull'
getting generated in one of its function 'write_int()'. However, I
couldn't find the way this 'gcc_tg.o' file is generated.

For your reference I have attached this object file with this mail.
(To escape the mailAdmin, I have renamed it as .TXT)

I did linked this 'gcc_tg.o' file with the simple "hello world"
program as below:
arm-eabi-gcc -mcpu=cortex-a9 hello.c gcc_tg.o -Wl,-wrap,exit
-Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort

and while debugging the output of this, after 'main()', I was able to
"nexti" into 'wrap_exit()' and then into 'wrap_int()' functions where
ultimately GDB failed to execute 'smull' command with error message:
====================================
sim: MULTIPLY64 - INVALID ARGUMENTS

*** EXIT code 6
====================================

I hope this will help to reproduce the problem at your end as well.

Regards,
Jayant


[-- Attachment #2: gcc_tg.txt --]
[-- Type: text/plain, Size: 2304 bytes --]

\x7fELF\x01\x01\x01\0\0\0\0\0\0\0\0\0\x01\0(\0\x01\0\0\0\0\0\0\0\0\0\0\0à\x03\0\0\0\0\0\x054\0\0\0\0\0(\0\v\0\b\0\0H-é\x04°â\x10ÐMâ\x10\0\vå\x14\x10\vå\x100^[å\0\0Sã\b\0\0ª\x140^[å-  ã\0 Ãå\x140^[å\x010ƒâ\x140\vå\x100^[å\00câ\x100\vå\x100^[å	\0Sã
\0\0Ú\x10 ^[åg6\x06ãf6Fã“\x12ÃàC\x11 áÂ? á\x010cà\x03\0 á\x14\x10^[åáÿÿë\x14\0\vå\x10\x10^[åg6\x06ãf6Fã“!ÃàC! áÁ? á\x02 cà\x020 á\x031 á\x020ƒàƒ0 á\x01 càr0ïæ00ƒâ\x050Kå\x140^[å\x05 [å\0 Ãå\x140^[å\x010ƒâ\x140\vå\x140^[å\x03\0 á\x04ÐKâ\0ˆ½è\0H-é\x04°â0ÐMâ0\0\vå( Kâ\00\0ã\00@ã\x02À á\x0f\0“è\x0f\0Œè(0Kâ\x03\0 áþÿÿë\00 á( Kâ\x030‚à0\0^[å\x03\x10 á´ÿÿë\b\0\vå\b0^[å
  ã\0 Ãå\b0^[å\x010ƒâ\b0\vå\b ^[å(0Kâ\x020cà( Kâ\x01\0 ã\x02\x10 á\x03  áþÿÿë\00\0ã\00@ã\x01  ã\0 ƒå0\0^[åþÿÿë\0\0 á\00\0ã\00@ã\00“å\0\0Sãúÿÿ\x1a\x04ÐKâ\0ˆ½è\0H-é\x04°â0ÐMâ0\0\vå\00\0ã\00@ã\00“å\0\0Sã\x1d\0\0\x1a( Kâ\00\0ã\00@ã\x02À á\x0f\0“è\x0f\0Œè(0Kâ\x03\0 áþÿÿë\00 á( Kâ\x030‚à0\0^[å\x03\x10 á\x7fÿÿë\b\0\vå\b0^[å
  ã\0 Ãå\b0^[å\x010ƒâ\b0\vå\b ^[å(0Kâ\x020cà( Kâ\x01\0 ã\x02\x10 á\x03  áþÿÿë0\0^[åþÿÿë\0\0 á\00\0ã\00@ã\00“å\0\0Sãúÿÿ\x1a\x04ÐKâ\0ˆ½è\0H-é\x04°â\x01\0 ã\0\x10\0ã\0\x10@ã\x14  ãþÿÿëþÿÿë\0\0 á\00\0ã\00@ã\00“å\0\0Sãúÿÿ\x1aþÿÿë\0H-é\x04°â\x10ÐMâ\b\0\vå\f\x10\vå\x10 \vå\b\0^[å\f\x10^[å\x10 ^[åþÿÿë\00 á\x03\0 áþÿÿë\x01\0\0\0
*** EXIT code \0
*** EXIT code 4242
\0\0\0\0\0GCC: (GNU) 4.7-GNUARM-RZ_v13.01\0A8\0\0\0aeabi\0\x01.\0\0\0\x05Cortex-A9\0\x06
\aA\b\x01	\x02\x12\x04\x14\x01\x15\x01\x17\x03\x18\x01\x19\x01\x1a\x01\x1e\x06"\x01*\x01D\x01\0.symtab\0.strtab\0.shstrtab\0.rel.text\0.data\0.bss\0.rodata\0.comment\0.ARM.attributes\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x1f\0\0\0\x01\0\0\0\x06\0\0\0\0\0\0\04\0\0\0Ô\x02\0\0\0\0\0\0\0\0\0\0\x04\0\0\0\0\0\0\0^[\0\0\0	\0\0\0\0\0\0\0\0\0\0\0(\b\0\0Ø\0\0\0	\0\0\0\x01\0\0\0\x04\0\0\0\b\0\0\0%\0\0\0\x01\0\0\0\x03\0\0\0\0\0\0\0\b\x03\0\0\x04\0\0\0\0\0\0\0\0\0\0\0\x04\0\0\0\0\0\0\0+\0\0\0\b\0\0\0\x03\0\0\0\0\0\0\0\f\x03\0\0\x04\0\0\0\0\0\0\0\0\0\0\0\x04\0\0\0\0\0\0\00\0\0\0\x01\0\0\0\x02\0\0\0\0\0\0\0\f\x03\0\0(\0\0\0\0\0\0\0\0\0\0\0\x04\0\0\0\0\0\0\08\0\0\0\x01\0\0\00\0\0\0\0\0\0\04\x03\0\0!\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\x01\0\0\0A\0\0\0\x03\0\0p\0\0\0\0\0\0\0\0U\x03\0\09\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\x11\0\0\0\x03\0\0\0\0\0\0\0\0\0\0\0Ž\x03\0\0Q\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\x01\0\0\0\x02\0\0\0\0\0\0\0\0\0\0\0˜\x05\0\0Ð\x01\0\0
\0\0\0\x10\0\0\0\x04\0\0\0\x10\0\0\0	\0\0\0\x03\0\0\0\0\0\0\0\0\0\0\0h\a\0\0À\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\x04\0ñÿ\0\0\0\0\0\0\0\0\0\0\0\0\x03\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\x03\0\x03\0\0\0\0\0\0\0\0\0\0\0\0\0\x03\0\x04\0\f\0\0\0\0\0\0\0\0\0\0\0\0\0\x04\0\x0f\0\0\0\0\0\0\0\0\0\0\0\0\0\x04\0\f\0\0\0\0\0\0\0\0\0\0\0\0\0\x03\0!\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0$\0\0\0\0\0\0\0à\0\0\0\x02\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\x03\0\x05\0\f\0\0\0\0\0\0\0\0\0\0\0\0\0\x05\0.\0\0\0\0\0\0\0\0\0\0\0\0\0\x05\03\0\0\0\x10\0\0\0\0\0\0\0\0\0\x05\0\0\0\0\0\0\0\0\0\0\0\0\0\x03\0\x06\0\0\0\0\0\0\0\0\0\0\0\0\0\x03\0\a\08\0\0\0\0\0\0\0\x04\0\0\0\x11\0\x03\0D\0\0\0à\0\0\0À\0\0\0\x12\0\x01\0P\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0W\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0]\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0i\0\0\0 \x01\0\0Ä\0\0\0\x12\0\x01\0v\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0ƒ\0\0\0d\x02\0\0<\0\0\0\x12\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0£\0\0\0 \x02\0\04\0\0\0\x12\0\x01\0¯\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0»\0\0\0\0\0\0\0\0\0\0\0\x10\0\0\0\0testglue.c\0$d\0done_exit_message\0$a\0write_int\0.LC0\0.LC1\0___constval\0__wrap_exit\0strlen\0write\0__real_exit\0__wrap__exit\0__real__exit\0__wrap_abort\0__real_abort\0abort\0__wrap_main\0__real_main\0exit\0ô\0\0\0+\f\0\0ø\0\0\0,\f\0\0\x10\x01\0\0\x1c\x12\0\0d\x01\0\0\x1c\x13\0\0h\x01\0\0+\x06\0\0l\x01\0\0,\x06\0\0|\x01\0\0\x1c\x14\0\0„\x01\0\0+\x10\0\0ˆ\x01\0\0,\x10\0\0°\x01\0\0+\x06\0\0´\x01\0\0,\x06\0\0È\x01\0\0+\f\0\0Ì\x01\0\0,\f\0\0ä\x01\0\0\x1c\x12\0\08\x02\0\0\x1c\x13\0\0@\x02\0\0\x1c\x16\0\0H\x02\0\0+\x10\0\0L\x02\0\0,\x10\0\0p\x02\0\0+\r\0\0t\x02\0\0,\r\0\0|\x02\0\0\x1c\x13\0\0€\x02\0\0\x1c\x18\0\0ˆ\x02\0\0+\x10\0\0Œ\x02\0\0,\x10\0\0œ\x02\0\0\x1c\x19\0\0Ä\x02\0\0\x1c^[\0\0Ð\x02\0\0\x1c\x1c\0\0

Re: [PATCH ARM] Fixing problem of 32bit multiplication instruction 'smull'

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 727 bytes --]

On Thursday 16 May 2013 11:13:06 Jayant R. Sonar wrote:
> Hello Mike,
> 
> Thanks again for reviewing this patch as well.
> 
> >> could you make that into a test case ? :)
> 
> I really don't have a test case in C form as of now.

you posted a small asm snippet.  seems like that'd be sufficient ?

in the Blackfin sim tests, i often just have the script exit 0 or 1 depending 
on the result.  if a specific insn is not calculating the right value (or is 
hitting an error it shouldn't), seems like you could do the same ?

	# pseudo code as my arm asm is not strong
	r1 = <known good value>
	r2 = <some input known to trigger bug>
	r3 = <diff input>
	r4 = r2 * r3
	if r1 != r4
		bx fail
	bx pass
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]