Minres / CoreDSL

Xtext project to parse CoreDSL files
Apache License 2.0
14 stars 2 forks source link

Add Mnemonic field to CoreDSL Syntax #80

Closed PhilippvK closed 1 year ago

PhilippvK commented 1 year ago

Motivation

With CoreDSL 2 instruction names can not include dots (.) while there are many extensions using names with a dot (see first & second example). While this might be neglectible for HLS and ISS, it affects further integrations, such as disassembler-Generation.

Therefore I propose to add an optional mnemonic: field to the CoreDSL 2 syntax which can be used to provide the actual instruction name if the name used in CoreDSL does not match the real one.

In addition I oftern run into situation where combining multiple instructions (which minor differences in the encoding/behavior) into a single one (see second & third example below), which of course will end up having an invalid name for that instructions.

To deal with this sort of problem I would like to be able to use similar formating options as already allowed for the assembly: field. For dealing with more complex types of instructions (having non-trivial mappings between encoding/operands and names) we would need to come up with a more powerful variant of this feature (see third example)

Examples

Mnemonics with a dot: Custom Multiply-Accumulate (Pulp/CoreV)

Spec: https://github.com/openhwgroup/cv32e40p/blob/master/docs/source/instruction_set_extensions.rst#mac-encoding

Usage: cv.mac rd, rs1, rs2

Before:

CV_MAC {
  encoding: 7'b1001000 :: rs2[4:0] :: rs1[4:0] :: 3'b011 :: rd[4:0] :: 7'b0101011;
  assembly:"{name(rd)}, {name(rs1)}, {name(rs2)}";
  behavior: {
    signed<65> result = (signed)X[rs1] * (signed)X[rs2] + (signed)X[rd];
    if(rd != 0) X[rd] = result[31:0];
  }
}

Problems:

After:

CV_MAC {
  mnemonic: "cv.mac";
  encoding: 7'b1001000 :: rs2[4:0] :: rs1[4:0] :: 3'b011 :: rd[4:0] :: 7'b0101011;
  assembly:"{name(rd)}, {name(rs1)}, {name(rs2)}";
  behavior: {
    signed<65> result = (signed)X[rs1] * (signed)X[rs2] + (signed)X[rd];
    if(rd != 0) X[rd] = result[31:0];
  }
}

Potential problems:

Trivial mnemonic formatting: Vector Strided Segment Loads (RVV)

See: https://github.com/openhwgroup/cv32e40p/blob/master/docs/source/instruction_set_extensions.rst#mac-encoding

Usage: vlsseg<nfields>e<eew>.v vd, (rs1), rs2, vm

Details:

For now let's only consider nfields. (eew has non-trivial encoding)

Before (combined):

VLSSEGE64_V {
    encoding: nf[2:0] :: 1'b0 :: 2'b10 :: vm[0:0] :: rs2[4:0] :: rs1[4:0] :: 3'b111 :: vd[4:0] :: 7'b0000111;
    assembly:"{name(vd)}, {name(rs1)}, {name(vm)}";
    behavior: {
        unsigned<4> nfields = nf + 1;
        ... // call to external softvector lib
    }
}

Problems:

Before (separate):

VLSSEG1E64_V {
    encoding: 2'b00 :: 1'b0 :: 2'b10 :: vm[0:0] :: rs2[4:0] :: rs1[4:0] :: 3'b111 :: vd[4:0] :: 7'b0000111;
    assembly:"{name(vd)}, {name(rs1)}, {name(vm)}";
    behavior: {
        unsigned<4> nfields = 1;  // nf + 1
        ... // call to external softvector lib
    }
}
VLSSEG2E64_V { ... }
VLSSEG3E64_V { ... }
VLSSEG4E64_V { ... }
VLSSEG5E64_V { ... }
VLSSEG6E64_V { ... }
VLSSEG7E64_V { ... }
VLSSEG8E64_V { ... }

Problems:

After (combined):

VLSSEGE64_V {
    mnemonic: "vlsseg{nf+1}e64.v";
    encoding: nf[2:0] :: 1'b0 :: 2'b10 :: vm[0:0] :: rs2[4:0] :: rs1[4:0] :: 3'b111 :: vd[4:0] :: 7'b0000111;
    assembly:"{name(vd)}, {name(rs1)}, {name(vm)}";
    behavior: {
        unsigned<4> nfields = nf + 1;
        ... // call to external softvector lib
    }
}

Potential problems:

Non-trivial mnemonic formatting : Byte Unpacking (RVP)

Spec: https://github.com/riscv/riscv-p-spec/blob/master/P-ext-proposal.adoc#sunpkd810-sunpkd820-sunpkd830-sunpkd831-sunpkd832

Usage:

Details:

Before (combined):

SUNPKD8 { // or SUNPKD8XY
    encoding: 7'b1010110 :: code[4:0] :: rs1[4:0] :: 3'b000 :: rd[4:0] :: 7'b1110111;
    assembly:"{name(rs1)}, {name(rd)}";
    behavior: {
        if(rd != 0) {
            unsigned<32> rs1_val = X[rs1];
            if(code == 5'b01000) {  // SUNPKD810
                signed<8> rs1_val_hi = rs1_val[15:8];
                signed<8> rs1_val_lo = rs1_val[7:0];
            } else if (code == 5'b01001) {  // SUNPKD820
                signed<8> rs1_val_hi = rs1_val[23:16];
                signed<8> rs1_val_lo = rs1_val[7:0];
            } else if (code == 5'b01010) {  // SUNPKD830
                signed<8> rs1_val_hi = rs1_val[31:24];
                signed<8> rs1_val_lo = rs1_val[7:0];
            } else if (code == 5'b01011) {  // SUNPKD831
                signed<8> rs1_val_hi = rs1_val[31:24];
                signed<8> rs1_val_lo = rs1_val[15:8];
            } else if (code == 5'b10011) {  // SUNPKD832
                signed<8> rs1_val_hi = rs1_val[31:24];
                signed<8> rs1_val_lo = rs1_val[23:16];
            } else {
                raise(0, 2);  // Invalid instruction
            }
            X[rd] = (signed<16>)rs1_val_hi :: (unsigned<16>)(signed<16>)rs1_val_lo;
        }
    }
}

Problems:

Before (separate):

SUNPKD810 {
    encoding: 7'b1010110 :: 5'b01000 :: rs1[4:0] :: 3'b000 :: rd[4:0] :: 7'b1110111;
    assembly:"{name(rs1)}, {name(rd)}";
    behavior: {
        if(rd != 0) {
            unsigned<32> rs1_val = X[rs1];
            signed<8> rs1_val_hi = rs1_val[15:8];
            signed<8> rs1_val_lo = rs1_val[7:0];
            X[rd] = (signed<16>)rs1_val_hi :: (unsigned<16>)(signed<16>)rs1_val_lo;
        }
    }
}
SUNPKD820 { ... }
SUNPKD830 { ... }
SUNPKD831 { ... }
SUNPKD832 { ... }

Problems:

Before (separate + helper function):

unsigned<32> sunpkd8_helper(unsigned<32> data, unsigned<5> code) {
    if(code == 5'b01000) {  // SUNPKD810
        signed<8> rs1_val_hi = rs1_val[15:8];
        signed<8> rs1_val_lo = rs1_val[7:0];
    } else if (code == 5'b01001) {  // SUNPKD820
        signed<8> rs1_val_hi = rs1_val[23:16];
        signed<8> rs1_val_lo = rs1_val[7:0];
    } else if (code == 5'b01010) {  // SUNPKD830
        signed<8> rs1_val_hi = rs1_val[31:24];
        signed<8> rs1_val_lo = rs1_val[7:0];
    } else if (code == 5'b01011) {  // SUNPKD831
        signed<8> rs1_val_hi = rs1_val[31:24];
        signed<8> rs1_val_lo = rs1_val[15:8];
    } else if (code == 5'b10011) {  // SUNPKD832
        signed<8> rs1_val_hi = rs1_val[31:24];
        signed<8> rs1_val_lo = rs1_val[23:16];
    } else {
        raise(0, 2);  // Invalid instruction
    }
    return (signed<16>)rs1_val_hi :: (unsigned<16>)(signed<16>)rs1_val_lo;
}

SUNPKD810 {
    encoding: 7'b1010110 :: 5'b01000 :: rs1[4:0] :: 3'b000 :: rd[4:0] :: 7'b1110111;
    assembly:"{name(rs1)}, {name(rd)}";
    behavior: {
        if(rd != 0) {
            X[rd] = sunpkd8_helper(X[rs1], 5'b01000)
        }
    }
}
SUNPKD820 { ... }
SUNPKD830 { ... }
SUNPKD831 { ... }
SUNPKD832 { ... }

Problem:

After (combined only):

string decode_xy(unsigend<5> code) {
    if(code == 5'b01000) {  // SUNPKD810
        return "10";
    } else if (code == 5'b01001) {  // SUNPKD820
        return "20";
    } else if (code == 5'b01010) {  // SUNPKD830
        return "30";
    } else if (code == 5'b01011) {  // SUNPKD831
        return "31";
    } else if (code == 5'b10011) {  // SUNPKD832
        return "32";
    } else {
        return "";
    }
}

SUNPKD8 { // or SUNPKD8XY
    mnemonic: "sunpkd8{decode_xy(code)}"
    encoding: 7'b1010110 :: code[4:0] :: rs1[4:0] :: 3'b000 :: rd[4:0] :: 7'b1110111;
    assembly:"{name(rs1)}, {name(rd)}";
    behavior: {
        if(rd != 0) {
            unsigned<32> rs1_val = X[rs1];
            if(code == 5'b01000) {  // SUNPKD810
                signed<8> rs1_val_hi = rs1_val[15:8];
                signed<8> rs1_val_lo = rs1_val[7:0];
            } else if (code == 5'b01001) {  // SUNPKD820
                signed<8> rs1_val_hi = rs1_val[23:16];
                signed<8> rs1_val_lo = rs1_val[7:0];
            } else if (code == 5'b01010) {  // SUNPKD830
                signed<8> rs1_val_hi = rs1_val[31:24];
                signed<8> rs1_val_lo = rs1_val[7:0];
            } else if (code == 5'b01011) {  // SUNPKD831
                signed<8> rs1_val_hi = rs1_val[31:24];
                signed<8> rs1_val_lo = rs1_val[15:8];
            } else if (code == 5'b10011) {  // SUNPKD832
                signed<8> rs1_val_hi = rs1_val[31:24];
                signed<8> rs1_val_lo = rs1_val[23:16];
            } else {
                raise(0, 2);  // Invalid instruction
            }
            X[rd] = (signed<16>)rs1_val_hi :: (unsigned<16>)(signed<16>)rs1_val_lo;
        }
    }
}

Potential problems:

PhilippvK commented 1 year ago

CC @eyck @jopperm @wysiwyng @DanMueGri

jopperm commented 1 year ago

Neat! I agree that this would be important to support more real-world extensions. @PhilippvK can you present this proposal in the WG call on 13/3?

PhilippvK commented 1 year ago

I can present it next Monday!

jopperm commented 1 year ago

I talked with @AtomCrafty about allowing dots in the instruction name. Having two kinds of identifiers is actually not possible because the lexer cannot distinguish them when tokenizing the input. We could

a) allow strings for the name

instructions {
  ADDI {...}
  "cv.mac" {...}
}

or

b) parse ID-with-dots as expressions (which would require some processing in the downstream tools, though the effort should be manageable, as instruction names are not references anywhere else).

Thoughts?

PhilippvK commented 1 year ago

I figured out that the XCoreVMem ISA extension has some more good examples for the Mnemonic use-case: https://cv32e40p.readthedocs.io/en/latest/instruction_set_extensions.html#load-operations

They have register-register load/store instructions with and without Post-Increment which share the same mnemonic but use differend assembly arguments for differentiation (beside the encoding of course):

Using the mnemonic as CoreDSL identifier leads to two instructions with the same name which can lead to issues. In my opinion these identifiers should be unique, even if only the encoding is used on the backend side. Hence I would prefer to use the following syntax:

CV_LB_rr {
    mnemonic: "cv.lb";
    assembly: "{name(rd)}, {name(rs2)}({name(rs1)})";
    ....
}
CV_LB_rr_inc {
    mnemonic: "cv.lb";
    assembly: "{name(rd)}, {name(rs2)}({name(rs1)!})";
    ....
}

Would you now agree, that the mnemonic: field would be a better solution, than just allowing . in instruction identifiers? @jopperm @wysiwyng @eyck

eyck commented 1 year ago

There is no objection wrt. to the possibility if specifying a mnemonic. But I'm a bit hessitant to add keywords to the language itself. Why not extending the assembly to either take a string (as of now) or take a list of strings enclosed in braces. The example would look like:

CV_LB_rr {
    assembly: {"cv.lb", "{name(rd)}, {name(rs2)}({name(rs1)})"};
    ....
}
CV_LB_rr_inc {
    assembly: { "cv.lb", "{name(rd)}, {name(rs2)}({name(rs1)!})"};
    ....
}

Since the frontend is not validating the content of the string this would be a minor change in the grammar and validation framework (@AtomCrafty correct my if I'm wrong)

AtomCrafty commented 1 year ago

I believe that would be a pure grammar change. The frontend doesn't perform any validation on the assembly field.

PhilippvK commented 1 year ago

@eyck I am totally fine with that proposal as long as it is properly documented in the manual

jopperm commented 1 year ago

SGTM!

jopperm commented 1 year ago

Frontend support has been implemented. I'll leave this issue open to track the necessary changes to the spec.

jopperm commented 1 year ago

Diff