4ad / go.arm64

Go development tree for the arm64 port (historical).
BSD 3-Clause "New" or "Revised" License
17 stars 3 forks source link

liblink: use immediates for small-constant loads #6

Open mwhudson opened 9 years ago

mwhudson commented 9 years ago

I have a fairly terrible script (http://pastebin.ubuntu.com/9481723/) that lets me compare the assembly of files like this http://pastebin.ubuntu.com/9521747/ to the disassembly provided by binutils. In this specific case, the disassembly is http://pastebin.ubuntu.com/9521771/, which shows that certain values are being misassembled (1,2,3,4,6,7,8,12,14,15,16,24,28,30,31,32 below 40 -- no obvious pattern to me!).

My best guess is that this is aclass returning the wrong thing for these values for some reason.

4ad commented 9 years ago

What's with the WORD at the beginning of the function?

mwhudson commented 9 years ago

It's how I find the object code in the .7 file. BTW, I first saw this with a similarly terrible script to do the same sort of thing with 7g (pipe 7g -S file.go into this https://gist.github.com/mwhudson/01923f69d75be25c3d5a), so I do think it's liblink this time.

On 15 December 2014 at 12:58, Aram Hăvărneanu notifications@github.com wrote:

What's with the WORD at the beginning of the function?

— Reply to this email directly or view it on GitHub https://github.com/4ad/go/issues/6#issuecomment-66936478.

4ad commented 9 years ago

This works fine, it just uses the more complex load-literal form. Basically it's an optimisation bug, we generate more complex code than we need to, but the code itself is fine (check the data at those addresses).

4ad commented 9 years ago

We will solve this eventually, because we want to generate efficient code, but for now it's fine as is as it works.

mwhudson commented 9 years ago

Aaah, ok, sorry for the noise.

4ad commented 9 years ago

If you see an obvious problem, please fix it though. I did look at this in the past but it didn't seem trivial to fix, perhaps I was wrong.

mwhudson commented 9 years ago

Well, so the problem is that the constants are those that get the class of C_ABCON (they are small numbers of the from 2^i-2^j, https://oeis.org/A023758). There are entries in Optab for both MOV C_ADDCON, C_REG and MOV C_BITCON, C_REG (which are compatible classes) but they are commented out and the corresponding cases in asmout() do not do the right thing.

Making cmp() think that C_ABCON is compatible with C_MOVCON fixes my cases, but isn't (aiui) valid in general.

I don't think I had quite realized just how wacky the A64 ISA was when it came to encoding integers until now!

mwhudson commented 9 years ago

I had a bit of a poke at this and think with the zoo of arm64 immediate encodings, a comprehensive implementation of always using an immediate encoding when possible would be impractical at in the current style where you call aclass without any kind of operation hint. But we can probably do better for the small immediates at least (an immediate that fits in 12 bits can go almost anywhere).

mwhudson commented 9 years ago

Attached branch implements that and helps with my test cases. It's late for me and it's possible I'm not thinking clearly enough to edit cmp() but it seems OK...

4ad commented 9 years ago

Note that this works fine with the old 7c and old 7l (without liblink), so I must have screwed something up when doing liblink.

Aram Hăvărneanu

4ad commented 9 years ago

The bulk majority of these problems have been fixed.