-
Notifications
You must be signed in to change notification settings - Fork 201
Parse encodings #24
Comments
I've been reading the codebase and think I'd be able to tackle this |
So the recipe system is currently only used with the RISC-V backend, am I right in thinking all the other backends are very much incomplete at this point? |
This encoding would surely have to be variable length? One ISA instruction doesn't necessarily correspond to one Cretonne opcode, eg RISC-V doesn't have IaddCarry so you need a branch and second add. |
Yes, at the moment only RISC-V has any encodings. The other ISAs are very incomplete. RISC-V is also very incomplete, just a little bit less. The encoding that gets printed out like pub struct Encoding {
recipe: u16,
bits: u16,
} The recipe is printed out as a name, pub static RECIPE_NAMES: [&'static str; 4] = [
"R",
"I",
"Rshamt",
"Iret",
]; The
It is the job of the legalizer to make sure that every Cretonne instruction in use maps to a single ISA opcode. For example, expand.legalize(
(a, c) << iadd_carry(x, y, c_in),
Rtl(
(a1, c1) << iadd_cout(x, y),
(a, c2) << iadd_cout(a1, c_in),
c << bor(c1, c2)
)) |
I am working on the register allocator right now, and I just pushed an extension to the encoding notation. It may now include value locations for the instruction's results:
The first line means that the result value
The value locations are optional, but if they are present, there should be exactly one per result value. Unassigned values are represented with
Lexical tokensThis introduces two new kinds of tokens that I would like to be able to use more generally:
I'd be happy to review and merge just a lexer patch. You don't have to do everything in one PR. |
For the hexadecimal bits, do you think I should follow what you did with |
Yes, that's a good idea. For example, a three-digit encoding is OK: |
I'm naming them
should we then aim to remove |
On Feb 22, 2017, at 16:23, angusholder ***@***.***> wrote:
I'm naming them HexSequence and Name if that sounds alright.
Yep, sounds good.
Given what you said in #47 <#47>
With these changes, the parser should stop accepting unquoted identifiers as function names.
should we then aim to remove Identifier, and expect alphanumeric sequence now to be a valid identifier?
No, I think we should keep `Identifier` because it is used for a number of context-sensitive keywords at the moment (`stack_slot`, `function`, etc). It’s possible we can switch back to real keywords after #47 is fixed, but let’s wait with that for now.
|
What do you think of adding the IsaSpec to the Parser or Context? parse_instruction() is going to need access to it to recognise the encoding strings. If I encounter an encoding at the start of an instruction and we've been given multiple IsaSpec's I assume that should be a parse error? |
What do you think of adding the IsaSpec to the Parser or Context? parse_instruction() is going to need access to it to recognise the encoding strings. If I encounter an encoding at the start of an instruction and we've been given multiple IsaSpec's I assume that should be a parse error?
It’s valid to have test files with no ISA spec, and it is valid to have multiple ISAs, see http://cretonne.readthedocs.io/en/latest/testing.html#file-tests
I think that encodings and register specs only make sense when a file has a single unique ISA. When there are none or multiple ISAs, just ignore the encodings. I don’t think we need to fail the parse.
It would make sense to add the `IsaSpec` to the `Parser`, I think.
|
This is almost done, but we still need to parse the value locations following the encoding:
The register value location The value locations are stored in the let reginfo = isa.register_info();
let regunit = reginfo.parse_regunit("x2").unwrap();
let loc = ValueLoc::Reg(regunit);
*ctx.function.locations.ensure(result) = loc; |
I'm working on this now. Should I make it a parse error if any value locations are specified when there isn't a unique isa? |
No, I think both encodings and value locations can be ignored if there's no unique ISA. I can imagine cases where you want to cut-and-paste test cases with encodings. It should be an error if the number of value locations doesn't match the number of results produced by the instruction. (Unless the number of value locations is 0, which is ok) |
This was fixed by @angusholder |
The
write_instruction()
function will print out the encoding of an instruction if it has been set:The parser should decode these annotations and fill out the
Function::encodings
table.Since the encoding recipe (
R
) is ISA-dependent, we probably can't support encodings in a test file with multiple ISAs.See the
DisplayEncoding
struct for details of the format.The text was updated successfully, but these errors were encountered: