llvm dev - Feb 2009 - [LLVMdev] list-td scheduler asserts on targets with implicitly defined registers

Hi,

I just switched to the 2.5 release branch and noticed that llc runs into the
following assert in ScheduleDAGList::ScheduleNodeTopDown() using our custom
backend:

    assert(!I->isAssignedRegDep() &&
           "The list-td scheduler doesn't yet support physreg
dependencies!");

It turns out that the register dependency concerns the condition code register
which is modeled as an implicitly defined register in the backend (the same
happens for e.g. for X86 when explicitly giving the -pre-RA-sched=list-td option
to llc).

My assumption is that the assert should exclude non-allocatable, implicitly
defined registers, which is checked in the attatched patch1.
This works fine for me, however on X86 the EFLAGS register is not marked
non-allocatable (patch2).
Is this intentional? Our backend handles condition codes pretty much like X86
and I remember I didn't get it to work without defining the
allocation_order_end() function in RegisterInfo.td
Anyway, I have no idea if this solution is ok for the general case, maybe the
implicit defs information should rather be put into the SDeps when they are
created?

Regards,
Christian

--







please ignore:

CONFIDENTIAL NOTICE: The contents of this message, including any attachments,
are confidential and are intended solely for the use of the person or entity to
whom the message was addressed. If you are not the intended recipient of this
message, please be advised that any dissemination, distribution, or use of the
contents of this message is strictly prohibited. If you received this message in
error, please notify the sender. Please also permanently delete all copies of
the original message and any attached documentation. Thank you.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch2_X86eflagsAllocatable.patch
Type: application/octet-stream
Size: 649 bytes
Desc: patch2_X86eflagsAllocatable.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090206/946c9800/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch1_physregAlloc.patch
Type: application/octet-stream
Size: 1641 bytes
Desc: patch1_physregAlloc.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090206/946c9800/attachment-0001.obj>

The best fix is to teach this scheduler how to deal with these  
dependencies. :-)

If you just want a check, I think it's easier to just check register  
class's copy cost. -1 means it's extremely expensive to copy registers  
in the particular register class.

Evan

On Feb 6, 2009, at 2:22 AM, Christian Sayer wrote:
> Hi,
>
> I just switched to the 2.5 release branch and noticed that llc runs  
> into the following assert in ScheduleDAGList::ScheduleNodeTopDown()  
> using our custom backend:
>
>    assert(!I->isAssignedRegDep() &&
>           "The list-td scheduler doesn't yet support physreg  
> dependencies!");
>
> It turns out that the register dependency concerns the condition  
> code register which is modeled as an implicitly defined register in  
> the backend (the same happens for e.g. for X86 when explicitly  
> giving the -pre-RA-sched=list-td option to llc).
>
> My assumption is that the assert should exclude non-allocatable,  
> implicitly defined registers, which is checked in the attatched  
> patch1.
> This works fine for me, however on X86 the EFLAGS register is not  
> marked non-allocatable (patch2).
> Is this intentional? Our backend handles condition codes pretty much  
> like X86 and I remember I didn't get it to work without defining the  
> allocation_order_end() function in RegisterInfo.td
> Anyway, I have no idea if this solution is ok for the general case,  
> maybe the implicit defs information should rather be put into the  
> SDeps when they are created?
>
> Regards,
> Christian
>
> --
>
>
>
>
>
>
>
> please ignore:
>
> CONFIDENTIAL NOTICE: The contents of this message, including any  
> attachments, are confidential and are intended solely for the use of  
> the person or entity to whom the message was addressed. If you are  
> not the intended recipient of this message, please be advised that  
> any dissemination, distribution, or use of the contents of this  
> message is strictly prohibited. If you received this message in  
> error, please notify the sender. Please also permanently delete all  
> copies of the original message and any attached documentation. Thank  
> you.
> < 
> patch2_X86eflagsAllocatable 
> .patch 
> > 
> < 
> patch1_physregAlloc 
> .patch>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

> The best fix is to teach this scheduler how to deal with these
> dependencies. :-)
>
> If you just want a check, I think it's easier to just check register
> class's copy cost. -1 means it's extremely expensive to copy
registers
> in the particular register class.
Evan,
I am not sure what you mean by "if you just want a check" - I was
trying to point out that for example the following very simple testcase makes
llc -march=x86 -pre-RA-sched=list-td crash :

define i32 @cctest(i32 %a, i32 %b) nounwind readnone {
entry:
        %not. = icmp sge i32 %a, %b             ; <i1> [#uses=1]
        %.0 = zext i1 %not. to i32              ; <i32> [#uses=1]
        ret i32 %.0
}

The assert() which triggers has been introduced since llvm 2.4.
So I assume that (at least for the time being) the assert condition has to be
made less restrictive. I also assume that this is ok because the dependency is
rather a 'flag' dependency than one to be resolved by the scheduler.
So the question would be if these assumptions are correct and if it is safe to
exclude implicitly defined physreg dependencies from the assert the way I
proposed. Furthermore, I was thinking that the exception should not apply on
allocatable registers, since the scheduler propably really cannot handle such.
When I realized that X86 EFLAGS is actually allocatable, I was wondering why,
but there is certainly a reason...

Best regards,
Christian

--







please ignore:

CONFIDENTIAL NOTICE: The contents of this message, including any attachments,
are confidential and are intended solely for the use of the person or entity to
whom the message was addressed. If you are not the intended recipient of this
message, please be advised that any dissemination, distribution, or use of the
contents of this message is strictly prohibited. If you received this message in
error, please notify the sender. Please also permanently delete all copies of
the original message and any attached documentation. Thank you.