Make exegesis conversion script use appropriate register class

[boomanaiden154]

This patch fixes the register class that the BHive to Exegesis conversion script uses. Currently it is only pulling in registers that don't have a REX encoding, which doesn't even include R9-R15. This patch fixes that behavior by using the more generic LLVM 64-bit GPR Register class, but specifically looking at registers that don't require a REX2 encoding, as there is no hardware in the wild yet that supports APX.

[boomanaiden154]

This does still include RIP, but that's an upstream LLVM issue.

[mtrofin]

Are we missing testing coverage here?

[boomanaiden154]

Are we missing testing coverage here?

There's no test coverage currently for this script. It would be best suited for lit/FileCheck based testing, but this is built with Bazel and it's less trivial to setup a lit test suite from within Bazel. It seems like Tensorflow has some infrastructure to do it (https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/glob_lit_test.bzl#L66) though if we want to add something eventually.

[ondrasej]

This does still include RIP, but that's an upstream LLVM issue.

Do we need to filter it out somewhere?

[boomanaiden154]

Do we need to filter it out somewhere?

It's not strictly necessary as it'll just end up getting encoded as something different (I believe %RAX). I've removed it as it definitely is weird though. I was originally intending to fix it on the LLVM side as there is a FIXME there, but given changing that causes 1000+ test failures and I'm not really familiar with that area of the backend, I'm not particularly interested in looking at it currently.

[ondrasej]

One minor comment, otherwise it looks OK.

[ondrasej]

It's not strictly necessary as it'll just end up getting encoded as something different (I believe %RAX). I've removed it as it definitely is weird though. I was originally intending to fix it on the LLVM side as there is a FIXME there, but given changing that causes 1000+ test failures and I'm not really familiar with that area of the backend, I'm not particularly interested in looking at it currently.

Gotcha. RIP can't be updated directly via MOV instructions (it doesn't have an index in the binary encoding), and IIRC it can only be read indirectly (via a LEA instruction or similar), and can only really be used in address computation.

I'm afraid that if we try to emit code that reads or writes RIP directly, weird things would happen (and LLVM would not catch them, because (IIRC) most of the validation happens on assembly parse time; onwards, the code just assumes that the MCInsts are correct, and does not check it.

[boomanaiden154]

Gotcha. RIP can't be updated directly via MOV instructions (it doesn't have an index in the binary encoding), and IIRC it can only be read indirectly (via a LEA instruction or similar), and can only really be used in address computation.

I'm afraid that if we try to emit code that reads or writes RIP directly, weird things would happen (and LLVM would not catch them, because (IIRC) most of the validation happens on assembly parse time; onwards, the code just assumes that the MCInsts are correct, and does not check it.

Right. It can't be updated, using those instructions. Once they end up getting written out, the instruction encoding doesn't even attempt to write to %RIP and instead writes to a different register (usually %RAX from what I've seen) due to some detail with instruction encodings that I'm forgetting currently.

It's a completely harmless thing (at least for us given all the registers are set to the same value), but a bit weird.

There is a machine code verification step that runs within llvm-exegesis (at least if assertions are enabled), but it doesn't catch this for whatever reason (probably because the instructions are only verified to be using the appropriate register class and %RIP is in the GP64 register class...). Might be something to look into adding at some point and I think would fix some open issues in LLVM on top of refactoring the register classes to be cleaner. I don't think that would be a trivial effort though.