[llvm-dev] Stuck with instruction in tablegen

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[llvm-dev] Stuck with instruction in tablegen

Robin Eklind via llvm-dev
Hi,

I'm trying to revive jacobly0's Z80 back-end (from
https://github.com/jacobly0/llvm-z80) and build it with a current
version of LLVM.

Apart from some interface changes, I'm stuck at building the tables.
Specifically, the generation of the DAG instruction selector causes an
assertion in the table generator:

     Assertion failed: Ops.size() >= NumSrcResults && "Didn't provide
enough results"

I tried to fix the problem by searched in the documentation and in other
table sources, but to no avail. Even stepping through that code in the
debugger (MatcherGen::EmitResultCode()) didn't help me to find the cause
of the problem. The instruction tables description are quite unique in
all the officially supported CPU models, I couldn't find any similar
code that could help me figuring out what is going wrong.

So I isolated the table sources to a minimum of two simple
base-instructions and attached these for reproduction of the problem
(not sure if the attachment gets through).

The table generation is done with:

     llvm-tblgen -gen-dag-isel -I=<path-to-llvm-include> -o=out.inc Z80.td

The first instruction in Z80InstrInfo.td is a unary instruction, that
does a rotate left of a byte in memory, resulting in the leftmost bit
being placed in the Carry flag - and changing the memory contents at the
pointer's address (in Z80: "RLC (HL)", I expect).
The table generation completes without errors with just this instruction
(by commenting the last line in Z80InstrInfo.td).

The second one is a binary instruction, adding the contents of a pointer
and a register, with results in register A and F (Z80: "ADD A,(HL)").

A is an 8-bit register. The Z80 ALU can only do 8 bit arithmetic with
that register and another param. The two instructions don't seem to
require a specific register for the address in the table class, so the
RegisterInfo table is not defined so far in the minimized version

The problem is in Z80InstrInfo.td, in the following section of
"BinOp8RF", which is used by the "defm" line that generates the "add"
instruction pattern:

     multiclass BinOp8RF<Prefix prefix, bits<3> opcode, string mnemonic,
                         bit compare = 0> {
       let isCompare = compare, Defs = [A, F], Uses = [A] in {
         def 8ap : I8 <prefix, {0b10, opcode, 0b110}, mnemonic, "\ta,
$src", "",
                       (outs), (ins   ptr:$src),
                       [(set A, F,
                            (Z80add_flag
                                 A, (i8 (load   iPTR:$src))))]>;
       }
     }

and the parameters:

     def SDTBinOpRF  : SDTypeProfile<2, 2, [SDTCisInt<0>,
                                            SDTCisFlag<1>,
                                            SDTCisSameAs<2, 0>,
                                            SDTCisSameAs<3, 0>]>;

     def Z80add_flag      : SDNode<"Z80ISD::ADD",     SDTBinOpRF,
[SDNPCommutative]>;


The unary instruction builds fine, which can be checked by commenting
out the generation of the binary instruction in the last line of the
attached Z80InstrInfo.td.

The log in Github indicates that this section was present for over a
year, so I assume that it did work at some time in the past.

My attempts to fixing the instruction failed with other errors, pointing
specifically to the specific problems I introduced with the change, e.g.
parameter number mismatch or wrong type, so above (and attached) is the
unmodified version.

Any help is appreciated!

Michael



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Z80-min-instruction-table-assertion.zip (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Stuck with instruction in tablegen

Robin Eklind via llvm-dev
To the best of my knowledge, having 2 explicit physical registers(in your case A and F) in the output of an instruction isn't supported correctly by tablegen and never has been. Maybe it previously appeared to work well enough to pass build?

This is why X86 has the pattern commented out here and implements custom handling in X86ISelDAGToDAG.cpp.

// EAX,EDX = EAX*GR32
let Defs = [EAX,EDX,EFLAGS], Uses = [EAX], hasSideEffects = 0 in
def MUL32r : I<0xF7, MRM4r, (outs),  (ins GR32:$src),
               "mul{l}\t$src",
               [/*(set EAX, EDX, EFLAGS, (X86umul_flag EAX, GR32:$src))*/]>,
               OpSize32, Sched<[WriteIMul]>;
// RAX,RDX = RAX*GR64
let Defs = [RAX,RDX,EFLAGS], Uses = [RAX], hasSideEffects = 0 indef MUL64r : RI<0xF7, MRM4r, (outs), (ins GR64:$src),
                "mul{q}\t$src",
                [/*(set RAX, RDX, EFLAGS, (X86umul_flag RAX, GR64:$src))*/]>,
                Sched<[WriteIMul64]>;


I vaguely recall trying to fix tablegen probably at least 5 years ago, but didn't succeed. Looking back at the code now I can tell you the two places in DAGISelMatcherGen.cpp that call HasOneImplicitDefWithKnownVT are definitely unable to handle multiple implicit def destinations in a pattern.

~Craig


On Mon, Jul 9, 2018 at 9:30 PM Michael Stellmann via llvm-dev <[hidden email]> wrote:
Hi,

I'm trying to revive jacobly0's Z80 back-end (from
https://github.com/jacobly0/llvm-z80) and build it with a current
version of LLVM.

Apart from some interface changes, I'm stuck at building the tables.
Specifically, the generation of the DAG instruction selector causes an
assertion in the table generator:

     Assertion failed: Ops.size() >= NumSrcResults && "Didn't provide
enough results"

I tried to fix the problem by searched in the documentation and in other
table sources, but to no avail. Even stepping through that code in the
debugger (MatcherGen::EmitResultCode()) didn't help me to find the cause
of the problem. The instruction tables description are quite unique in
all the officially supported CPU models, I couldn't find any similar
code that could help me figuring out what is going wrong.

So I isolated the table sources to a minimum of two simple
base-instructions and attached these for reproduction of the problem
(not sure if the attachment gets through).

The table generation is done with:

     llvm-tblgen -gen-dag-isel -I=<path-to-llvm-include> -o=out.inc Z80.td

The first instruction in Z80InstrInfo.td is a unary instruction, that
does a rotate left of a byte in memory, resulting in the leftmost bit
being placed in the Carry flag - and changing the memory contents at the
pointer's address (in Z80: "RLC (HL)", I expect).
The table generation completes without errors with just this instruction
(by commenting the last line in Z80InstrInfo.td).

The second one is a binary instruction, adding the contents of a pointer
and a register, with results in register A and F (Z80: "ADD A,(HL)").

A is an 8-bit register. The Z80 ALU can only do 8 bit arithmetic with
that register and another param. The two instructions don't seem to
require a specific register for the address in the table class, so the
RegisterInfo table is not defined so far in the minimized version

The problem is in Z80InstrInfo.td, in the following section of
"BinOp8RF", which is used by the "defm" line that generates the "add"
instruction pattern:

     multiclass BinOp8RF<Prefix prefix, bits<3> opcode, string mnemonic,
                         bit compare = 0> {
       let isCompare = compare, Defs = [A, F], Uses = [A] in {
         def 8ap : I8 <prefix, {0b10, opcode, 0b110}, mnemonic, "\ta,
$src", "",
                       (outs), (ins   ptr:$src),
                       [(set A, F,
                            (Z80add_flag
                                 A, (i8 (load   iPTR:$src))))]>;
       }
     }

and the parameters:

     def SDTBinOpRF  : SDTypeProfile<2, 2, [SDTCisInt<0>,
                                            SDTCisFlag<1>,
                                            SDTCisSameAs<2, 0>,
                                            SDTCisSameAs<3, 0>]>;

     def Z80add_flag      : SDNode<"Z80ISD::ADD",     SDTBinOpRF,
[SDNPCommutative]>;


The unary instruction builds fine, which can be checked by commenting
out the generation of the binary instruction in the last line of the
attached Z80InstrInfo.td.

The log in Github indicates that this section was present for over a
year, so I assume that it did work at some time in the past.

My attempts to fixing the instruction failed with other errors, pointing
specifically to the specific problems I introduced with the change, e.g.
parameter number mismatch or wrong type, so above (and attached) is the
unmodified version.

Any help is appreciated!

Michael


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Stuck with instruction in tablegen

Robin Eklind via llvm-dev
Thanks Craig for your quick reply and pointing me to a direction!
 
So even when finding a syntax that tablegen would accept, it would not lead to the desired result.
 
Will look at the X86 code you mentioned below and try to understand how it's done.
 
Although the X86 seems to be the "closest" to the Z80 regarding the instruction set (and, as far as I can see, the only officially supported CISC CPU), it seems also to be the most complex backend to get started with. But I'm commited ;-)
 
Michael
 
Gesendet: Dienstag, 10. Juli 2018 um 08:21 Uhr
Von: "Craig Topper" <[hidden email]>
An: [hidden email]
Cc: llvm-dev <[hidden email]>
Betreff: Re: [llvm-dev] Stuck with instruction in tablegen
To the best of my knowledge, having 2 explicit physical registers(in your case A and F) in the output of an instruction isn't supported correctly by tablegen and never has been. Maybe it previously appeared to work well enough to pass build?
 
This is why X86 has the pattern commented out here and implements custom handling in X86ISelDAGToDAG.cpp.
 
// EAX,EDX = EAX*GR32
let Defs = [EAX,EDX,EFLAGS], Uses = [EAX], hasSideEffects = 0 in
def MUL32r : I<0xF7, MRM4r, (outs),  (ins GR32:$src),
               "mul{l}\t$src",
               [/*(set EAX, EDX, EFLAGS, (X86umul_flag EAX, GR32:$src))*/]>,
               OpSize32, Sched<[WriteIMul]>;
// RAX,RDX = RAX*GR64
let Defs = [RAX,RDX,EFLAGS], Uses = [RAX], hasSideEffects = 0 indef MUL64r : RI<0xF7, MRM4r, (outs), (ins GR64:$src),
                "mul{q}\t$src",
                [/*(set RAX, RDX, EFLAGS, (X86umul_flag RAX, GR64:$src))*/]>,
                Sched<[WriteIMul64]>;
 
 
I vaguely recall trying to fix tablegen probably at least 5 years ago, but didn't succeed. Looking back at the code now I can tell you the two places in DAGISelMatcherGen.cpp that call HasOneImplicitDefWithKnownVT are definitely unable to handle multiple implicit def destinations in a pattern.
 
~Craig
 
On Mon, Jul 9, 2018 at 9:30 PM Michael Stellmann via llvm-dev <[hidden email]> wrote:
Hi,

I'm trying to revive jacobly0's Z80 back-end (from
https://github.com/jacobly0/llvm-z80) and build it with a current
version of LLVM.

Apart from some interface changes, I'm stuck at building the tables.
Specifically, the generation of the DAG instruction selector causes an
assertion in the table generator:

     Assertion failed: Ops.size() >= NumSrcResults && "Didn't provide
enough results"

I tried to fix the problem by searched in the documentation and in other
table sources, but to no avail. Even stepping through that code in the
debugger (MatcherGen::EmitResultCode()) didn't help me to find the cause
of the problem. The instruction tables description are quite unique in
all the officially supported CPU models, I couldn't find any similar
code that could help me figuring out what is going wrong.

So I isolated the table sources to a minimum of two simple
base-instructions and attached these for reproduction of the problem
(not sure if the attachment gets through).

The table generation is done with:

     llvm-tblgen -gen-dag-isel -I=<path-to-llvm-include> -o=out.inc Z80.td

The first instruction in Z80InstrInfo.td is a unary instruction, that
does a rotate left of a byte in memory, resulting in the leftmost bit
being placed in the Carry flag - and changing the memory contents at the
pointer's address (in Z80: "RLC (HL)", I expect).
The table generation completes without errors with just this instruction
(by commenting the last line in Z80InstrInfo.td).

The second one is a binary instruction, adding the contents of a pointer
and a register, with results in register A and F (Z80: "ADD A,(HL)").

A is an 8-bit register. The Z80 ALU can only do 8 bit arithmetic with
that register and another param. The two instructions don't seem to
require a specific register for the address in the table class, so the
RegisterInfo table is not defined so far in the minimized version

The problem is in Z80InstrInfo.td, in the following section of
"BinOp8RF", which is used by the "defm" line that generates the "add"
instruction pattern:

     multiclass BinOp8RF<Prefix prefix, bits<3> opcode, string mnemonic,
                         bit compare = 0> {
       let isCompare = compare, Defs = [A, F], Uses = [A] in {
         def 8ap : I8 <prefix, {0b10, opcode, 0b110}, mnemonic, "\ta,
$src", "",
                       (outs), (ins   ptr:$src),
                       [(set A, F,
                            (Z80add_flag
                                 A, (i8 (load   iPTR:$src))))]>;
       }
     }

and the parameters:

     def SDTBinOpRF  : SDTypeProfile<2, 2, [SDTCisInt<0>,
                                            SDTCisFlag<1>,
                                            SDTCisSameAs<2, 0>,
                                            SDTCisSameAs<3, 0>]>;

     def Z80add_flag      : SDNode<"Z80ISD::ADD",     SDTBinOpRF,
[SDNPCommutative]>;


The unary instruction builds fine, which can be checked by commenting
out the generation of the binary instruction in the last line of the
attached Z80InstrInfo.td.

The log in Github indicates that this section was present for over a
year, so I assume that it did work at some time in the past.

My attempts to fixing the instruction failed with other errors, pointing
specifically to the specific problems I introduced with the change, e.g.
parameter number mismatch or wrong type, so above (and attached) is the
unmodified version.

Any help is appreciated!

Michael


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Stuck with instruction in tablegen

Robin Eklind via llvm-dev
It seems like not really the same case to me. Z80 "F" is similar to the condition code register in many other machines, and is set implicitly by all or most instructions.

In the case of x86 EAX and EDX, I wonder if you could define the pair as a pseudo-register, in the same way that .. well .. H and L make HL etc.


On Tue, Jul 10, 2018 at 12:17 AM, Michael Stellmann via llvm-dev <[hidden email]> wrote:
Thanks Craig for your quick reply and pointing me to a direction!
 
So even when finding a syntax that tablegen would accept, it would not lead to the desired result.
 
Will look at the X86 code you mentioned below and try to understand how it's done.
 
Although the X86 seems to be the "closest" to the Z80 regarding the instruction set (and, as far as I can see, the only officially supported CISC CPU), it seems also to be the most complex backend to get started with. But I'm commited ;-)
 
Michael
 
Gesendet: Dienstag, 10. Juli 2018 um 08:21 Uhr
Von: "Craig Topper" <[hidden email]>
An: [hidden email]
Cc: llvm-dev <[hidden email]>
Betreff: Re: [llvm-dev] Stuck with instruction in tablegen
To the best of my knowledge, having 2 explicit physical registers(in your case A and F) in the output of an instruction isn't supported correctly by tablegen and never has been. Maybe it previously appeared to work well enough to pass build?
 
This is why X86 has the pattern commented out here and implements custom handling in X86ISelDAGToDAG.cpp.
 
// EAX,EDX = EAX*GR32
let Defs = [EAX,EDX,EFLAGS], Uses = [EAX], hasSideEffects = 0 in
def MUL32r : I<0xF7, MRM4r, (outs),  (ins GR32:$src),
               "mul{l}\t$src",
               [/*(set EAX, EDX, EFLAGS, (X86umul_flag EAX, GR32:$src))*/]>,
               OpSize32, Sched<[WriteIMul]>;
// RAX,RDX = RAX*GR64
let Defs = [RAX,RDX,EFLAGS], Uses = [RAX], hasSideEffects = 0 indef MUL64r : RI<0xF7, MRM4r, (outs), (ins GR64:$src),
                "mul{q}\t$src",
                [/*(set RAX, RDX, EFLAGS, (X86umul_flag RAX, GR64:$src))*/]>,
                Sched<[WriteIMul64]>;
 
 
I vaguely recall trying to fix tablegen probably at least 5 years ago, but didn't succeed. Looking back at the code now I can tell you the two places in DAGISelMatcherGen.cpp that call HasOneImplicitDefWithKnownVT are definitely unable to handle multiple implicit def destinations in a pattern.
 
~Craig
 
On Mon, Jul 9, 2018 at 9:30 PM Michael Stellmann via llvm-dev <[hidden email]> wrote:
Hi,

I'm trying to revive jacobly0's Z80 back-end (from
https://github.com/jacobly0/llvm-z80) and build it with a current
version of LLVM.

Apart from some interface changes, I'm stuck at building the tables.
Specifically, the generation of the DAG instruction selector causes an
assertion in the table generator:

     Assertion failed: Ops.size() >= NumSrcResults && "Didn't provide
enough results"

I tried to fix the problem by searched in the documentation and in other
table sources, but to no avail. Even stepping through that code in the
debugger (MatcherGen::EmitResultCode()) didn't help me to find the cause
of the problem. The instruction tables description are quite unique in
all the officially supported CPU models, I couldn't find any similar
code that could help me figuring out what is going wrong.

So I isolated the table sources to a minimum of two simple
base-instructions and attached these for reproduction of the problem
(not sure if the attachment gets through).

The table generation is done with:

     llvm-tblgen -gen-dag-isel -I=<path-to-llvm-include> -o=out.inc Z80.td

The first instruction in Z80InstrInfo.td is a unary instruction, that
does a rotate left of a byte in memory, resulting in the leftmost bit
being placed in the Carry flag - and changing the memory contents at the
pointer's address (in Z80: "RLC (HL)", I expect).
The table generation completes without errors with just this instruction
(by commenting the last line in Z80InstrInfo.td).

The second one is a binary instruction, adding the contents of a pointer
and a register, with results in register A and F (Z80: "ADD A,(HL)").

A is an 8-bit register. The Z80 ALU can only do 8 bit arithmetic with
that register and another param. The two instructions don't seem to
require a specific register for the address in the table class, so the
RegisterInfo table is not defined so far in the minimized version

The problem is in Z80InstrInfo.td, in the following section of
"BinOp8RF", which is used by the "defm" line that generates the "add"
instruction pattern:

     multiclass BinOp8RF<Prefix prefix, bits<3> opcode, string mnemonic,
                         bit compare = 0> {
       let isCompare = compare, Defs = [A, F], Uses = [A] in {
         def 8ap : I8 <prefix, {0b10, opcode, 0b110}, mnemonic, "\ta,
$src", "",
                       (outs), (ins   ptr:$src),
                       [(set A, F,
                            (Z80add_flag
                                 A, (i8 (load   iPTR:$src))))]>;
       }
     }

and the parameters:

     def SDTBinOpRF  : SDTypeProfile<2, 2, [SDTCisInt<0>,
                                            SDTCisFlag<1>,
                                            SDTCisSameAs<2, 0>,
                                            SDTCisSameAs<3, 0>]>;

     def Z80add_flag      : SDNode<"Z80ISD::ADD",     SDTBinOpRF,
[SDNPCommutative]>;


The unary instruction builds fine, which can be checked by commenting
out the generation of the binary instruction in the last line of the
attached Z80InstrInfo.td.

The log in Github indicates that this section was present for over a
year, so I assume that it did work at some time in the past.

My attempts to fixing the instruction failed with other errors, pointing
specifically to the specific problems I introduced with the change, e.g.
parameter number mismatch or wrong type, so above (and attached) is the
unmodified version.

Any help is appreciated!

Michael


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Stuck with instruction in tablegen

Robin Eklind via llvm-dev
Yes, the "F" register equals to X86's "EFLAGS" register, and indeed, the Z80 treats AF (i.e. the 8 bit accumulator and the 8 bit flags) as a 16-bit pair, just like HL, being composed of the registers H and L - and in fact, in the original Z80RegisterInfo.td, A and F are aliased to the pair AF, just like HL.
 
If I understood you correctly, you are proposing changing

  [(set A, F, ...

to

  [(set AF, ...

This would affect the SDTypeProfile from

def SDTBinOpRF  : SDTypeProfile<2, 2, [SDTCisInt<0>,
                                       SDTCisFlag<1>,
                                       SDTCisSameAs<2, 0>,
                                       SDTCisSameAs<3, 0>]>;

i.e. a (8-bit) integer register "A", and a (8-bit) flag register "F" to merge into a (16-bit) integer register "AF"

def SDTBinOpRF  : SDTypeProfile<1, 2, [SDTCisInt<0>,
                                       SDTCisSameAs<1, 0>,
                                       SDTCisSameAs<2, 0>]>;

which would omit the "SDTCisFlag".

I'm not that much into LLVM yet (started with that only ~1 week ago) but I would expect that this would have to be dealt with somewhere else in the code, right?

Michael
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev