llvm dev - Nov 2008 - [LLVMdev] Multi-instruction patterns, tablegen and chains

Hi Dan,
> Having tblgen pretend that the MOVE isn't the root seems a bit
> counter-intuitive though.
I didn't really mean making RD the root, but rather telling tablegen that RD
is the "primary" node, corresponding to the input pattern. This would
allow
the properties of the input rd SDNode be properly transferred to the output RD
node instead of the MOVE node. I doubt this is a very elegant solution, nor
will it work in the general case, but this is what I meant anyway.
> I think it would be useful to have a way to explicitly tell
> tblgen which node(s) in the output pattern to hook up the chain(s)
> to. I believe this is related to a problem David Greene is seeing,
> where he has a case involving multiple chains.
> A fully general approach would be to make chain operands explicit
> in patterns, and to invent some syntax for identifying chain
> results. It might look something like this:
> 
> def : Pat<(rd ch:$input_chain, imm:$addr)[$output_chain],
>            (MOVE (RD ch:$input_chain, imm:$addr)[$output_chain])>;
> 
> This would be a pretty big change though.
So you're basically allowing for a way to specify outputs beyond the first
output in a dag? That does make sense, and might be useful for other cases to,
perhaps?

I won't be able to implement this, however, since I'm only prototyping
just
yet...
> An alternative that's a bit less ambitious would be to have
> tablegen search the output pattern for nodes which are declared
> to support chain operands/results, and if it finds exactly one
> such node, use that node to hook up all the chain
> operands/results. If it finds more than one, it could issue
> an error. This would probably be simpler and less invasive,
> and probably enough for your specific example, though it
> wouldn't be as general-purpose.
Currently, I think that  the code supports having multiple nodes with chain
just fine (when I hook up a pattern to my RD instruction, I get an Emit
function that hooks up the chain to both RD and MOVE. I think it might even
join the two output chains together, not exactly sure about that. Not sure if
this is really what I want, but it would work.

So, the main thing needed is some way of telling that an instruction is
chained, other than giving it a pattern. Using hasCtrlDep for this makes
sense, but I'm afraid I won't be having the time nor expertise to
implement
this... I'd be glad to test it though :-)

Gr.

Matthijs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20081104/ae57b8d4/attachment.sig>

On Nov 4, 2008, at 12:12 AM, Matthijs Kooijman wrote:
> Hi Dan,
>
>> Having tblgen pretend that the MOVE isn't the root seems a bit
>> counter-intuitive though.
> I didn't really mean making RD the root, but rather telling tablegen  
> that RD
> is the "primary" node, corresponding to the input pattern. This  
> would allow
> the properties of the input rd SDNode be properly transferred to the  
> output RD
> node instead of the MOVE node. I doubt this is a very elegant  
> solution, nor
> will it work in the general case, but this is what I meant anyway.
Ok. I agree, not very elegant, but you may be able to come up with  
something
good enough for whatever prototyping you're doing without too much extra
effort.
>
>
>> I think it would be useful to have a way to explicitly tell
>> tblgen which node(s) in the output pattern to hook up the chain(s)
>> to. I believe this is related to a problem David Greene is seeing,
>> where he has a case involving multiple chains.
>> A fully general approach would be to make chain operands explicit
>> in patterns, and to invent some syntax for identifying chain
>> results. It might look something like this:
>>
>> def : Pat<(rd ch:$input_chain, imm:$addr)[$output_chain],
>>           (MOVE (RD ch:$input_chain, imm:$addr)[$output_chain])>;
>>
>> This would be a pretty big change though.
> So you're basically allowing for a way to specify outputs beyond the  
> first
> output in a dag? That does make sense, and might be useful for other  
> cases to,
> perhaps?
Yes, being able to work with nodes with multiple results with tablegen
patterns would be useful for a variety of things.
>
>
> I won't be able to implement this, however, since I'm only  
> prototyping just
> yet...
>
>
>> An alternative that's a bit less ambitious would be to have
>> tablegen search the output pattern for nodes which are declared
>> to support chain operands/results, and if it finds exactly one
>> such node, use that node to hook up all the chain
>> operands/results. If it finds more than one, it could issue
>> an error. This would probably be simpler and less invasive,
>> and probably enough for your specific example, though it
>> wouldn't be as general-purpose.
> Currently, I think that  the code supports having multiple nodes  
> with chain
> just fine (when I hook up a pattern to my RD instruction, I get an  
> Emit
> function that hooks up the chain to both RD and MOVE. I think it  
> might even
> join the two output chains together, not exactly sure about that.  
> Not sure if
> this is really what I want, but it would work.
But from your earlier description, MOVE didn't need a chain, only RD  
did.
So even there, it sounds like tablegen isn't doing exactly what you  
want.

Dan

Hi Dan,
> But from your earlier description, MOVE didn't need a chain, only RD 
did.
> So even there, it sounds like tablegen isn't doing exactly what you 
want.
Yeah, but I think an extra chain is something I can live with for now, but a
missing chain is a problem :-)

Anyway, I found another somewhat-working workaround. I created dummy SDNode
defs that the SDNPHasChain set to the correct value, and a pattern for the
MOVE and RD instructions that used those SDNodes. Because the nodes were dummy
(referring ISD::DELETED_NODE), I could be sure they would never match, while
they would still transfer the correct chain property to the instruction
itself.

This took a considerate amount of tweaking, because even for these dummy nodes
tblgen insisted that the types would be correct. This resulted in the
following SDNodes:

	def dummy_rd    : SDNode<"ISD::DELETED_NODE", SDTRead,
[SDNPHasChain]>;
	def dummy_move  : SDNode<"ISD::DELETED_NODE", SDTRead, []>;
	def dummy_intop : SDNode<"ISD::DELETED_NODE", SDTIntLeaf, []>;

and these patterns for the MOVE and RD instructions resp.

	(set DatapathReg:$dst, (dummy_move dummy_intop:$src))

	(set Bus:$dst, (dummy_rd dummy_intop:$addr))

Now, my multi-instruction pattern correctly matches and produces the proper
chain results:

	def          : Pat< (rd imm:$addr),
											(MOVE (RD imm:$addr))
										>;

So that got me where I thought I wanted to be, but then the scheduler started
messing up things. There are some constraints between the MOVE and RD
instructions, which are probably best modeled by connecting by a Flag.

Adding a SDNPInFlag property to the dummy_rd and SDNPOutFlag to dummy_move
didn't quite help: The code generated for the Pat was unaffected, the code
generated for the Instruction pattern for RD was now broken, I suspect that RD
was producing too many values (ie, there was no getTargetNode to handle this
case yet).

In the end, I settled for doing custom selection copying most code from my
last attempt and manually adding a Flag operand/result to that code. This
seems to work and actually produce working code (for the two instructions
I've
implemented so far...).

Now I'll be continuing to see if other funkyness of our architecture can be
properly modeled by LLVM...

Thanks for the pointers!

Gr.

Matthijs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20081110/d4aeb929/attachment.sig>