Closed Quuxplusone closed 14 years ago
Quuxplusone
commented
14 years ago | Bugzilla Link | PR6349 |
| Status | RESOLVED FIXED |
| Importance | P normal |
| Reported by | Rohit Pagariya (pagariya@cs.utah.edu) |
| Reported on | 2010-02-18 17:54:05 -0800 |
| Last modified on | 2010-03-06 05:42:13 -0800 |
| Version | trunk |
| Hardware | PC Linux |
| CC | anton@korobeynikov.info, llvm-bugs@lists.llvm.org, regehr@cs.utah.edu |
| Fixed by commit(s) | |
| Attachments | |
| Blocks | |
| Blocked by | |
| See also |
Quuxplusone
commented
14 years ago I have the following assembler code:
mov.w #0, 2(r1)
mov.w #0, 0(r1)
.LBB1_1: ; %for.cond
; =>This Inner Loop Header: Depth=1
cmp.w #2, 0(r1)
jge .LBB1_3
; BB#2: ; %for.body
; in Loop: Header=BB1_1 Depth=1
mov.w 0(r1), r12
rla.w r12
mov.w #-695, &g_4(r12)
add.w #1, 0(r1)
jmp .LBB1_1
so, it's pretty obvious, there is a store. Confirmed on mspsim as well.
Next time please attach LLVM IR and/or resulting assembler as well.Anton- Rohit has convinced me that there's something funny going on here. May
we quickly revisit this example?
Let's start with some simpler C code:
volatile long g_4[2];
void foo (void)
{
int i;
for (i = 0; i < 2; i++) g_4[i] = 0x8B72FD49L;
}
Clang emits asm that looks fine:
regehr@john-home:~$ clang -ccc-host-triple msp430-elf -O1 -S -o - msp3.c -fomit-
frame-pointer
foo:
mov.w #0, r12
.LBB1_1:
mov.w r12, r13
rla.w r13
rla.w r13
add.w #1, r12
mov.w #-29838, &g_4+2(r13)
mov.w #-695, &g_4(r13)
cmp.w #2, r12
jne .LBB1_1
ret
However, assembling and then disassembling gives the following:
regehr@john-home:~$ msp430-gcc -mmcu=msp430x1611 msp3.s -c
regehr@john-home:~$ msp430-objdump -d msp3.o
00000000 <foo>:
0: 0c 43 clr r12
2: 0d 4c mov r12, r13
4: 0d 5d rla r13
6: 0d 5d rla r13
8: 1c 53 inc r12
a: b2 40 72 8b mov #-29838,&0x0000 ;#0x8b72
e: 00 00
10: b2 40 49 fd mov #-695, &0x0000 ;#0xfd49
14: 00 00
16: 2c 93 cmp #2, r12 ;r3 As==10
18: f4 23 jnz $-22 ;abs 0x2
1a: 30 41 ret
Something strange has happened: the instructions at 0xa and 0x10 do not mention
our index variable r13. Is it possible that clang's assembly output is somehow
not conforming to the expectations of msp430-as?
If we do the equivalent experiment using mspgcc, of course the index variable
is mentioned in the disassembly and we get this:
00000000 <foo>:
0: 0e 43 clr r14
2: 3d 40 00 00 mov #0, r13 ;#0x0000
6: 0f 4e mov r14, r15
8: 0f 5f rla r15
a: 0f 5f rla r15
c: 0f 5d add r13, r15
e: bf 40 49 fd mov #-695, 0(r15) ;#0xfd49, 0x0000(r15)
12: 00 00
14: bf 40 72 8b mov #-29838,2(r15) ;#0x8b72, 0x0002(r15)
18: 02 00
1a: 1e 53 inc r14
1c: 2e 93 cmp #2, r14 ;r3 As==10
1e: f3 3b jl $-24 ;abs 0x6
20: 30 41 retI have found other instances of this particular problem. One common thread is that all are related to arrays and use indexed addressing mode for accessing the variable.
Quuxplusone
commented
14 years ago Hrm, sounds like msp430-as bug, if it does not support this expression - it should emit an error, not silently miscompile it...
Anyone brave enough to hack on binutils sources? Or, maybe, there will be better to hook llvm asm parser machinery here? :)
Quuxplusone
commented
14 years ago (In reply to comment #4)
> Hrm, sounds like msp430-as bug, if it does not support this expression - it
> should emit an error, not silently miscompile it...
>
> Anyone brave enough to hack on binutils sources? Or, maybe, there will be
> better to hook llvm asm parser machinery here? :)
The llvm asm stuff would be great-- but that sounds like perhaps a lot of work?
Probably there are complications with the interrupt table, etc.
Rohit, please take a quick look at msp430-as and see what is going on.Anton is there any documentation for msp430-as that you were using when you decided what kind of syntax to emit?
Quuxplusone
commented
14 years ago (In reply to comment #6)
> Anton is there any documentation for msp430-as that you were using when you
> decided what kind of syntax to emit?
I used official TI documentation.
I'm expecting that stuff like &g+2(r15) is valid in indexed mode since g+2 is a
link time immediate. Basically, assembler should just emit an relocation here
instead of imm field and linker should put a real constant here. And it indeed
does so if you drop the register!This is what I think is happening. Correct me if I am wrong.
First, some background on different addressing modes in msp430.
(Section 3.3, Pg 43 msp430x1xx family user guide)
There are 7 for source while 4 for destination, which we are interested in.
They are
1. Register
Eg. mov r1, r2
2. Indexed
Eg. mov r1, 0(r2)
3. Symbolic
Eg. mov foo, bar
Here 'foo' and 'bar' are symbols.
4. Absolute.
Eg. mov r1, &foo
Consider the following generated instruction:
mov.w #-695, &g_4(r12)
The destination addressing mode doesn't match any of the four modes described
above. In fact, it is absolute indexed addressing mode (which is not supported).
My theory is that when the assembler see that instruction, it considers it to
be absolute mode and accordingly generates instructions. The linker then
relocates the reference to the appropriate symbol.
The msp430 assembly language tools user guide did not give me any insight about
the problem. I then looked closely at the assembly output of llvm and mspgcc.
After some experiments with mspgcc assembly, I observed that it does not use
the absolute indexed addressing mode.
So the correct assembly instead of 'mov.w #-695, &g_4(r12)' would be
something like
mov #g_4, r8 /* Move address of g_4 to r8 */
mov #-695, 0(r8) /* Add offset 0 to address of g_4 and then move -695 to it */
When the upper word of the 32-bit integer is to be accessed, the offset could
become 2.(In reply to comment #8)
> Consider the following generated instruction:
> mov.w #-695, &g_4(r12)
>
> The destination addressing mode doesn't match any of the four modes described
It's definitely indexed mode. And it should be supported - In the final binary
you'd have something like
mov.w #-695, 0xAAAA(r12), where 0xAAAA is an adress of g_4 global. It does not matter at which point this immediate occur - for processor everything is a number.
> My theory is that when the assembler see that instruction, it considers it to
> be absolute mode and accordingly generates instructions.
There should be 2 possibilities:
1. Assembler should support sush construction and act accordingly
2. It should reject this construction
In any case - it should not silently miscompile the stuff (well, this might be
pointless - we all know about extremely good 'quality' of msp430 toolchain).Below: the Crossworks for msp430 output for my function.
Rohit: since this is obviously legal code, you need to look at why binutils
isn't supporting it.
_foo
MOV.W #0, R15
@0
MOV.W R15, R14
ADD.W R14, R14
ADD.W R14, R14
MOV.W #0xfd49, _g_4(R14)
MOV.W #0x8b72, _g_4 + 2(R14)
@1
ADD.W #1, R15
CMP.W #2, R15
JL @0
@2
RETBy the way I don't understand why all of these compilers have failed to transform the induction variable so that 4 is added to it each time through the loop. This would shorten the critical path by 2 instructions.
Quuxplusone
commented
14 years ago (In reply to comment #11)
> By the way I don't understand why all of these compilers have failed to
> transform the induction variable so that 4 is added to it each time through
the
> loop. This would shorten the critical path by 2 instructions.
Well, I know why LLVM didn't do this - it assumes that multiplication by 2, 4,
etc. is better done via shifts. This might be fixed.(In reply to comment #10)
> Below: the Crossworks for msp430 output for my function.
>
> Rohit: since this is obviously legal code, you need to look at why binutils
> isn't supporting it.
>
> MOV.W #0xfd49, _g_4(R14)
> MOV.W #0x8b72, _g_4 + 2(R14)
In the above example, _g_4 is a link time constant and will be compiled
correctly. I can confirm this with llvm-msp430, msp430-gcc and TI Code Composer
Studio IDE.
But when you prefix an '&' sign to the symbol, it becomes absolute addressing
mode and is miscompiled by msp430-as silently. TI CCS compiler fails with an
'unexpected trailing operands' error suggesting more arguments are provided
than needed. This suggests that when the assembler sees the '&' symbol, it
assumes absolute addressing mode.
When I remove the '&' prefixed to g_4 from the llvm assembly and compile as
usual, there are no errors in the code.
mov.w #-695, g_4(r12)
mov.w #-29838, g_4+2(r12)
is compiled to
mov #-695, 4356(r12) ;#0xfd49, 0x1104(r12)
mov #-29838,4358(r12) ;#0x8b72, 0x1106(r12)
which is what we expect.(In reply to comment #13)>
> When I remove the '&' prefixed to g_4 from the llvm assembly and compile as
> usual, there are no errors in the code.
>
> mov.w #-695, g_4(r12)
> mov.w #-29838, g_4+2(r12)
>
> is compiled to
>
> mov #-695, 4356(r12) ;#0xfd49, 0x1104(r12)
> mov #-29838,4358(r12) ;#0x8b72, 0x1106(r12)
>
> which is what we expect.
Hrm, how weird.... Ok, I understand the problem and will fix it.Fixed in http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20100301/097373.html
please verify