llvm dev - Jan 2009 - [LLVMdev] Use two ComplexPatterns (possible bug of TableGen?)

On Wednesday 14 January 2009 18:59:03 Brandner Florian
wrote:
> I have a patch against llvm 2.4 that fixes this issue, but did not have
> the time to post the patch here. I'll do so by tomorrow.
here is the patch, still against llvm 2.4. I had a short look on trunk, but it 
seems that there are several conflicts. Maybe a tablgen expert should have a 
look at this - I also do not know if there are changes needed for the 
FastISelEmitter.

I hope this is a starting point for you,
florian

ps: We have a working compiler here that relies on these changes, so it does 
not seem to break anything, at least for our private backend.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: multiple-complexpatterns.patch
Type: text/x-patch
Size: 2958 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090115/fe4ad532/attachment.bin>

On Jan 15, 2009, at 12:50 AM, Brandner wrote:
> On Wednesday 14 January 2009 18:59:03 Brandner Florian wrote:
>> I have a patch against llvm 2.4 that fixes this issue, but did not  
>> have
>> the time to post the patch here. I'll do so by tomorrow.
>
> here is the patch, still against llvm 2.4. I had a short look on  
> trunk, but it
> seems that there are several conflicts. Maybe a tablgen expert  
> should have a
> look at this - I also do not know if there are changes needed for the
> FastISelEmitter.
>
> I hope this is a starting point for you,
> florian
>
> ps: We have a working compiler here that relies on these changes, so  
> it does
> not seem to break anything, at least for our private backend.

Hi Florian,

Thanks for the patch! I applied the parts of the patch that rename the
CPTmp variables:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090112/072209.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090112/072210.html

However, I didn't apply this part:

-      if (InstPatNode && InstPatNode->getOperator()->getName() ==
"set") {
+      if (InstPatNode && !InstPatNode->isLeaf() &&
+          InstPatNode->getOperator()->getName() == "set") {

because I'm unsure what it's for. When is a "set" node a leaf?

BTW, the FastISelEmitter should be ignoring any instructions that
have ComplexPatterns, so there shouldn't be a problem there. Also,
it's only enabled for x86 right now.

Dan

Hi Dan,

thank you for applying the patch.
> However, I didn't apply this part:
>
> -      if (InstPatNode &&
InstPatNode->getOperator()->getName() => "set") {
> +      if (InstPatNode && !InstPatNode->isLeaf() &&
> +          InstPatNode->getOperator()->getName() == "set")
{
>
> because I'm unsure what it's for. When is a "set" node a
leaf?
exactly it never is, however InstPatNode may well be a leaf. in that case 
calling getOperator is not valid, because it asserts on !isLeaf:

Record *getOperator() const { assert(!isLeaf()); return Operator; }

so this check definitely should be there. but it is true, i should have split 
the patch into two pieces as it is not related to the ComplexPattern issue. 
sorry for that.

florian