[EXT] Re: [aarch64-port-dev ] RFR(XXS): 8222412: AARCH64: lse atomics encoding is not accepting zr as source

[Andrew Haley]

On 4/16/19 3:15 PM, Derek White wrote:
> Hi Andrew,
> 
> I asked Dmitrij to look at instruction encodings. 
> 
> The instruction encodings are the basis to the back-end of the
> aarch64 port. There have been at least 10 latent bugs [1] reported
> in these encodings. These were unimportant because they weren't
> used, until they were.

Firstly: some instructions use r31 for sp, some for zr. It is not a
bug when the assembler does not specify which, but asserts when either
is used. That is deliberate. It is because I do not want the assembler
to be untested: the first time anyone tries to use those instructions
for real we'll get an assert, and then the programmer will have to
actually check that the right thing happens.

So, let's have a look at what we've had.

JDK-8210578. Fixed by adinn, actually a bug.

JDK-8191769. Fixed by dpochepk. actually a bug, but unused.

JDK-8221995. Fixed by dpochepk. actually a bug, but only applies to
             CASP, which we are unlikely ever to use.

JDK-8214961. Fixed by dpochepk. not a bug

JDK-8205474. Fixed by dpochepk. actually a bug, but unused.

JDK-8194256. Fixed by dpochepk. actually a bug, but unused.

JDK-8222412. not a bug

JDK-8202395. Fixed by dpochepk. actually a bug, but unused.

JDK-8201185. Fixed by dpochepk. not a bug

JDK-8221765. actually a bug

JDK-8195859. Fixed by adinn, actually a bug.

> These encoding bugs take little effort to find, less to review, and
> are a small investment to make future changes in the aarch64 port
> easier. The effort is much lower to find and fix them at once,
> instead of piecemeal over a few years. Furthermore, this isn't an
> open universe of issues - there is a well-defined, fix set of
> instructions to check.

I'd rather the ones where ZR/SP was deliberately left unspecified were
not fixed, really. I don't really mind if someone checks *really*
*carefully*, but I don't have the time.

> We've appreciated your detailed and thoughtful reviews immensely,
> and I might agree that these bugs aren't worth *your* time
> reviewing. But I think some of your concerns reflect a lack of
> review capacity in the aarch64 port.

I disagree.

> The rest of the JDK happily
> accepts checkins that are nothing more than fixing punctuation in
> code comments.
> 
> In the meantime, we can batch up the encoding bugs in a bundle or two. 

One other thing that you might like to consider. When I first wrote
the assembler I checked all of the instructions against the equivalent
code generated by GAS. The script is here:

http://hg.openjdk.java.net/aarch64-port/jdk8/file/9a781f9c1338/test/aarch64-asmtest.py

Unfortunately, as the assembler was extended this script was never
updated, so it only checks the encoding of the core instructions. If
you wanted to do something really useful you could extend this script
to test the encoding of all the instructions we generate, including
the SIMD ones. It would provide a truly independent verification of
the assembler. And, once it's done, you can submit a single patch
containing all of the fixes you find.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671