Jakob Stoklund Olesen
2013-Mar-21 18:26 UTC
[LLVMdev] Simpler types in TableGen isel patterns
Currently, instruction selection patterns are defined like this:
def : Pat<(and (not GR32:$src1), GR32:$src2),
(ANDN32rr GR32:$src1, GR32:$src2)>;
def : Pat<(and (not GR64:$src1), GR64:$src2),
(ANDN64rr GR64:$src1, GR64:$src2)>;
TableGen infers the types of $src1 and $src2 from the specified register
classes, and that is the only purpose of the register classes in a pattern like
that. SelectionDAG doesn't really understand register classes, it only uses
types.
If I try to constrain the register class in a pattern, like this:
def : Pat<(and (not GR32_ABCD:$src1), GR32_ABCD:$src2),
(ANDN32rr GR32_ABCD:$src1, GR32_ABCD:$src2)>;
I get completely ignored. SelectionDAG's InstrEmitter will still use the
GR32 register class that was assigned to the i32 type.
When using register classes as proxies for types, it also becomes very difficult
to support more than one legal type in a register class. If I were to entertain
the heretic notion that an f32 might fit in a 32-bit register:
def GR32 : RegisterClass<"X86", [i32, f32], 32, ...
TableGen explodes with a thousand type inference errors.
I think in most cases it would be much simpler and safer to specify pattern
types directly:
def : Pat<(and (not i32:$src1), i32:$src2),
(ANDN32rr i32:$src1, i32:$src2)>;
def : Pat<(and (not i64:$src1), i64:$src2),
(ANDN64rr i64:$src1, i64:$src2)>;
This doesn't lose any type checking because the register classes of the
instructions in the output pattern are still checked. It avoids the problem
where type inference makes it impractical to add types to a register class to
model instruction set extensions, and it makes it clear that patterns operate on
types, not register classes.
I'll add support for this syntax unless someone can think of a reason I
shouldn't. The notation using register classes gets to stay, I'm not
removing it.
/jakob
----- Original Message -----> From: "Jakob Stoklund Olesen" <stoklund at 2pi.dk> > To: llvmdev at cs.uiuc.edu, llvmdev at cs.uiuc.edu, llvmdev at cs.uiuc.edu, llvmdev at cs.uiuc.edu > Sent: Thursday, March 21, 2013 1:26:25 PM > Subject: [LLVMdev] Simpler types in TableGen isel patterns > > Currently, instruction selection patterns are defined like this: > > def : Pat<(and (not GR32:$src1), GR32:$src2), > (ANDN32rr GR32:$src1, GR32:$src2)>; > def : Pat<(and (not GR64:$src1), GR64:$src2), > (ANDN64rr GR64:$src1, GR64:$src2)>; > > TableGen infers the types of $src1 and $src2 from the specified > register classes, and that is the only purpose of the register > classes in a pattern like that. SelectionDAG doesn't really > understand register classes, it only uses types. > > If I try to constrain the register class in a pattern, like this: > > def : Pat<(and (not GR32_ABCD:$src1), GR32_ABCD:$src2), > (ANDN32rr GR32_ABCD:$src1, GR32_ABCD:$src2)>; > > I get completely ignored. SelectionDAG's InstrEmitter will still use > the GR32 register class that was assigned to the i32 type. > > When using register classes as proxies for types, it also becomes > very difficult to support more than one legal type in a register > class. If I were to entertain the heretic notion that an f32 might > fit in a 32-bit register: > > def GR32 : RegisterClass<"X86", [i32, f32], 32, ... > > TableGen explodes with a thousand type inference errors. > > > I think in most cases it would be much simpler and safer to specify > pattern types directly: > > def : Pat<(and (not i32:$src1), i32:$src2), > (ANDN32rr i32:$src1, i32:$src2)>; > def : Pat<(and (not i64:$src1), i64:$src2), > (ANDN64rr i64:$src1, i64:$src2)>; > > This doesn't lose any type checking because the register classes of > the instructions in the output pattern are still checked. It avoids > the problem where type inference makes it impractical to add types > to a register class to model instruction set extensions, and it > makes it clear that patterns operate on types, not register classes. > > I'll add support for this syntax unless someone can think of a reason > I shouldn't. The notation using register classes gets to stay, I'm > not removing it.I think this is a good idea. Is there a reason why we need the types again in the expansion? Would it be easy to make the syntax like this? def : Pat<(and (not i64:$src1), i64:$src2), (ANDN64rr $src1, $src2)>; Thanks again, Hal> > /jakob > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
This sounds great! I've been bitten in the past by trying to use a single class for multiple types. Would it make sense to extend this to all DAG patterns? If I have an instruction def: def ANDN64 : MyInst<(outs Reg64:$d), (ins Reg64:$a, Reg64:$b), "and.64 $d, $a, $b", [(set Reg64:$d, (and (not (Reg64:$a, Reg64:$b))))]>; would I now be able to write: def ANDN64 : MyInst<(outs Reg64:$d), (ins Reg64:$a, Reg64:$b), "and.64 $d, $a, $b", [(set i64:$d, (and (not (i64:$a, i64:$b))))]>; ? On Thu, Mar 21, 2013 at 3:00 PM, Hal Finkel <hfinkel at anl.gov> wrote:> ----- Original Message ----- > > From: "Jakob Stoklund Olesen" <stoklund at 2pi.dk> > > To: llvmdev at cs.uiuc.edu, llvmdev at cs.uiuc.edu, llvmdev at cs.uiuc.edu, > llvmdev at cs.uiuc.edu > > Sent: Thursday, March 21, 2013 1:26:25 PM > > Subject: [LLVMdev] Simpler types in TableGen isel patterns > > > > Currently, instruction selection patterns are defined like this: > > > > def : Pat<(and (not GR32:$src1), GR32:$src2), > > (ANDN32rr GR32:$src1, GR32:$src2)>; > > def : Pat<(and (not GR64:$src1), GR64:$src2), > > (ANDN64rr GR64:$src1, GR64:$src2)>; > > > > TableGen infers the types of $src1 and $src2 from the specified > > register classes, and that is the only purpose of the register > > classes in a pattern like that. SelectionDAG doesn't really > > understand register classes, it only uses types. > > > > If I try to constrain the register class in a pattern, like this: > > > > def : Pat<(and (not GR32_ABCD:$src1), GR32_ABCD:$src2), > > (ANDN32rr GR32_ABCD:$src1, GR32_ABCD:$src2)>; > > > > I get completely ignored. SelectionDAG's InstrEmitter will still use > > the GR32 register class that was assigned to the i32 type. > > > > When using register classes as proxies for types, it also becomes > > very difficult to support more than one legal type in a register > > class. If I were to entertain the heretic notion that an f32 might > > fit in a 32-bit register: > > > > def GR32 : RegisterClass<"X86", [i32, f32], 32, ... > > > > TableGen explodes with a thousand type inference errors. > > > > > > I think in most cases it would be much simpler and safer to specify > > pattern types directly: > > > > def : Pat<(and (not i32:$src1), i32:$src2), > > (ANDN32rr i32:$src1, i32:$src2)>; > > def : Pat<(and (not i64:$src1), i64:$src2), > > (ANDN64rr i64:$src1, i64:$src2)>; > > > > This doesn't lose any type checking because the register classes of > > the instructions in the output pattern are still checked. It avoids > > the problem where type inference makes it impractical to add types > > to a register class to model instruction set extensions, and it > > makes it clear that patterns operate on types, not register classes. > > > > I'll add support for this syntax unless someone can think of a reason > > I shouldn't. The notation using register classes gets to stay, I'm > > not removing it. > > I think this is a good idea. > > Is there a reason why we need the types again in the expansion? Would it > be easy to make the syntax like this? > > def : Pat<(and (not i64:$src1), i64:$src2), > (ANDN64rr $src1, $src2)>; > > Thanks again, > Hal > > > > > /jakob > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Thanks, Justin Holewinski -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130321/4b9a5913/attachment.html>
Jakob Stoklund Olesen
2013-Mar-21 19:59 UTC
[LLVMdev] Simpler types in TableGen isel patterns
On Mar 21, 2013, at 12:00 PM, Hal Finkel <hfinkel at anl.gov> wrote:> I think this is a good idea. > > Is there a reason why we need the types again in the expansion? Would it be easy to make the syntax like this? > > def : Pat<(and (not i64:$src1), i64:$src2), > (ANDN64rr $src1, $src2)>;That's just a quirk of the generic TableGen DAG syntax. The 'i64' is the actual operand, and '$src1' is the name of the operand. The name is optional, but the operand itself is not. It wouldn't be that hard to make the parser treat $src2 as ?:$src2 where '?' represents an UndefInit. /jakob
Hey Jakob, On Thu, Mar 21, 2013 at 2:26 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:> Currently, instruction selection patterns are defined like this: > > def : Pat<(and (not GR32:$src1), GR32:$src2), > (ANDN32rr GR32:$src1, GR32:$src2)>; > def : Pat<(and (not GR64:$src1), GR64:$src2), > (ANDN64rr GR64:$src1, GR64:$src2)>; > > TableGen infers the types of $src1 and $src2 from the specified register > classes, and that is the only purpose of the register classes in a pattern > like that. SelectionDAG doesn't really understand register classes, it only > uses types. > > If I try to constrain the register class in a pattern, like this: > > def : Pat<(and (not GR32_ABCD:$src1), GR32_ABCD:$src2), > (ANDN32rr GR32_ABCD:$src1, GR32_ABCD:$src2)>; > > I get completely ignored. SelectionDAG's InstrEmitter will still use the > GR32 register class that was assigned to the i32 type. > > When using register classes as proxies for types, it also becomes very > difficult to support more than one legal type in a register class. If I > were to entertain the heretic notion that an f32 might fit in a 32-bit > register: > > def GR32 : RegisterClass<"X86", [i32, f32], 32, ... > > TableGen explodes with a thousand type inference errors. > > > I think in most cases it would be much simpler and safer to specify > pattern types directly: > > def : Pat<(and (not i32:$src1), i32:$src2), > (ANDN32rr i32:$src1, i32:$src2)>; > def : Pat<(and (not i64:$src1), i64:$src2), > (ANDN64rr i64:$src1, i64:$src2)>; > > This doesn't lose any type checking because the register classes of the > instructions in the output pattern are still checked. It avoids the problem > where type inference makes it impractical to add types to a register class > to model instruction set extensions, and it makes it clear that patterns > operate on types, not register classes. > > I'll add support for this syntax unless someone can think of a reason I > shouldn't. The notation using register classes gets to stay, I'm not > removing it. >Shooting from the hip... let's say we had a predicated execution architecture which had a general integer register class and a mask register class. Both classes contain i32 as a member and the instructions that operate on those two register classes are mutually exclusive. A little example: Integer AND ========iand %i0, %i1 Mask AND =======mand %m0, %m1 I'm wondering if having a general pattern could cause issues there. A pattern such as this could match both register classes, leading to a bad operand in the instruction produced and an assembler error. def : Pat<(and (not i32:$src1), i32:$src2), (ANDN32rr i32:$src1, i32:$src2)>; This shouldn't currently be an issue, but I am trying to look far down the road. Let's say that predicated execution becomes commonplace down that road and it's now more beneficial to select patterns on register classes, rather than types. To be clear, this is more a thought experiment than a critique. Making your change won't impact my work and I don't feel strongly for, nor against, this change. -Cameron -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130321/aee951dc/attachment.html>
On Mar 21, 2013, at 11:26 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:> I think in most cases it would be much simpler and safer to specify pattern types directly: > > def : Pat<(and (not i32:$src1), i32:$src2), > (ANDN32rr i32:$src1, i32:$src2)>; > def : Pat<(and (not i64:$src1), i64:$src2), > (ANDN64rr i64:$src1, i64:$src2)>; > > This doesn't lose any type checking because the register classes of the instructions in the output pattern are still checked. It avoids the problem where type inference makes it impractical to add types to a register class to model instruction set extensions, and it makes it clear that patterns operate on types, not register classes.This makes a huge amount of sense to me. Besides your motivating problem, writing an isel pattern that matches things in terms of register classes doesn't make *any* sense: the input graph has type labels that are MVTs. If you want to get *really* crazy, the ideal type system for dag nodes would be to allow either an EVT or a register class: after isel (and after some custom dag combines) the types of nodes should be register classes, since they are more specific than an MVT (or EVT). -Chris
Jakob Stoklund Olesen
2013-Mar-21 21:25 UTC
[LLVMdev] Simpler types in TableGen isel patterns
On Mar 21, 2013, at 1:39 PM, Chris Lattner <clattner at apple.com> wrote:> On Mar 21, 2013, at 11:26 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote: >> I think in most cases it would be much simpler and safer to specify pattern types directly: >> >> def : Pat<(and (not i32:$src1), i32:$src2), >> (ANDN32rr i32:$src1, i32:$src2)>; >> def : Pat<(and (not i64:$src1), i64:$src2), >> (ANDN64rr i64:$src1, i64:$src2)>; >> >> This doesn't lose any type checking because the register classes of the instructions in the output pattern are still checked. It avoids the problem where type inference makes it impractical to add types to a register class to model instruction set extensions, and it makes it clear that patterns operate on types, not register classes. > > This makes a huge amount of sense to me. Besides your motivating problem, writing an isel pattern that matches things in terms of register classes doesn't make *any* sense: the input graph has type labels that are MVTs.Exactly. The register classes are misleading.> If you want to get *really* crazy, the ideal type system for dag nodes would be to allow either an EVT or a register class: after isel (and after some custom dag combines) the types of nodes should be register classes, since they are more specific than an MVT (or EVT).Yes, that is also the direction I would like to go. We have started taking baby steps in this direction with the 'untyped' MVT. The register class for an untyped DAG edge is determined by the defining instruction alone. TLI.getRegClassFor() is not consulted. To clarify how register classes are computed today: - For a typed edge, the RC is computed as the intersection of TLI.getRegClassFor(type) and the register constraints imposed by all the virtual register's uses and defs. - For an untyped edge, the RC is the intersection of the def constraint and all the uses' constraints. Thanks, /jakob
Jakob Stoklund Olesen
2013-Mar-21 21:38 UTC
[LLVMdev] Simpler types in TableGen isel patterns
On Mar 21, 2013, at 1:06 PM, Cameron McInally <cameron.mcinally at nyu.edu> wrote:> Shooting from the hip... let's say we had a predicated execution architecture which had a general integer register class and a mask register class. Both classes contain i32 as a member and the instructions that operate on those two register classes are mutually exclusive. > > A little example: > > Integer AND > ========> iand %i0, %i1 > > Mask AND > =======> mand %m0, %m1 > > I'm wondering if having a general pattern could cause issues there. A pattern such as this could match both register classes, leading to a bad operand in the instruction produced and an assembler error. > > def : Pat<(and (not i32:$src1), i32:$src2), > (ANDN32rr i32:$src1, i32:$src2)>; > > This shouldn't currently be an issue, but I am trying to look far down the road. Let's say that predicated execution becomes commonplace down that road and it's now more beneficial to select patterns on register classes, rather than types. > > To be clear, this is more a thought experiment than a critique. Making your change won't impact my work and I don't feel strongly for, nor against, this change.The problem here is that SelectionDAG's type system doesn't always match architectures well. Sometimes you need more fine grained distinctions than offered by the types, like in your example. Sometimes you need something coarser than types, like when the PowerPC floating point registers can hold both f32 and f64 types. The fabs and fneg instructions are effectively polymorphic, operating on both f32 and f64 values. Many modern architectures can perform integer arithmetic in their vector/float registers, but LLVM will always use the integer registers because of the type <-> register class mapping. This causes more cross-class copies than is necessary. To address these problems, I think we will need to support multiple patterns matching the same primitive typed operation. The output patterns can select different instructions with different operand register classes, and we need to solve a graph coloring problem to minimize the cross-class copies produced. Thanks, /jakob
On Thu, Mar 21, 2013 at 2:26 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:> Currently, instruction selection patterns are defined like this: > > def : Pat<(and (not GR32:$src1), GR32:$src2), > (ANDN32rr GR32:$src1, GR32:$src2)>; > def : Pat<(and (not GR64:$src1), GR64:$src2), > (ANDN64rr GR64:$src1, GR64:$src2)>; > > TableGen infers the types of $src1 and $src2 from the specified register > classes, and that is the only purpose of the register classes in a pattern > like that. SelectionDAG doesn't really understand register classes, it only > uses types. > > If I try to constrain the register class in a pattern, like this: > > def : Pat<(and (not GR32_ABCD:$src1), GR32_ABCD:$src2), > (ANDN32rr GR32_ABCD:$src1, GR32_ABCD:$src2)>; > > I get completely ignored. SelectionDAG's InstrEmitter will still use the > GR32 register class that was assigned to the i32 type. > > When using register classes as proxies for types, it also becomes very > difficult to support more than one legal type in a register class. If I > were to entertain the heretic notion that an f32 might fit in a 32-bit > register: > > def GR32 : RegisterClass<"X86", [i32, f32], 32, ... > > TableGen explodes with a thousand type inference errors. > >How come the Hexagone backend is able to get away with that then? def IntRegs : RegisterClass<"Hexagon", [i32,f32], 32, -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130323/a0557183/attachment.html>
Jakob Stoklund Olesen
2013-Mar-23 20:54 UTC
[LLVMdev] Simpler types in TableGen isel patterns
On Mar 23, 2013, at 1:50 PM, Francois Pichet <pichet2000 at gmail.com> wrote:> On Thu, Mar 21, 2013 at 2:26 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote: > When using register classes as proxies for types, it also becomes very difficult to support more than one legal type in a register class. If I were to entertain the heretic notion that an f32 might fit in a 32-bit register: > > def GR32 : RegisterClass<"X86", [i32, f32], 32, ... > > TableGen explodes with a thousand type inference errors. > > > How come the Hexagone backend is able to get away with that then? > > def IntRegs : RegisterClass<"Hexagon", [i32,f32], 32,They have to use explicit type casts everywhere: def rr : ALU32_rr<(outs IntRegs:$dst), (ins IntRegs:$b, IntRegs:$c), !strconcat("$dst = ", !strconcat(OpcStr, "($b, $c)")), [(set (i32 IntRegs:$dst), (OpNode (i32 IntRegs:$b), (i32 IntRegs:$c)))]>; /jakob -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130323/da2d1f93/attachment.html>