BranchInst problem

While adding support for branch instructions in my backend, I run into a
trouble. The code to handle branches looks like:

        void visitBranchInst(BranchInst& BI)
        {
            BB->addSuccessor (MBBMap[BI.getSuccessor(0)]);
            if (BI.isConditional())
                BB->addSuccessor (MBBMap[BI.getSuccessor(1)]);

            ...........
                BuildMI(BB, NM::GOTO, 1).addPCDisp(BI.getSuccessor(0));
                BuildMI(BB, NM::GOTO, 1).addPCDisp(BI.getSuccessor(1));
         }

The machine code after instruction selection is:

   entry (0x8681458):
        %reg1024 = load <fi#-1>
        %reg1025 = load <fi#-2>
        setcc %reg1024, %reg1025
        goto %disp(label then)
        goto %disp(label else)

   then (0x8681688):
        %reg1026 = + %reg1025, %reg1024
        %gr7 = move %reg1026
        return
        ....

which looks ok, but after register allocation and prolog/epilogue insertion, I
get this:

   entry (0x8681458):
        .....
        setcc %gr0, %gr1
        goto %disp(label then)
        goto %disp(label else)
        %ar6 = + %ar6, 1
        store %ar6, %gr0
        %ar6 = - %ar6, 1
        %ar6 = + %ar6, 2
        store %ar6, %gr1
        %ar6 = - %ar6, 2

The code after "goto" is disturbing. It looks like spill code, but it's not
going to be ever executed. Any ideas why it's generated? Is there any
additional information I should provide?

- Volodya

While adding support for branch instructions in my backend, I run into a
trouble. The code to handle branches looks like:
The machine code after instruction selection is:

   entry (0x8681458):
        %reg1024 = load <fi#-1>
        %reg1025 = load <fi#-2>
        setcc %reg1024, %reg1025
        goto %disp(label then)
        goto %disp(label else)

I assume that the two unconditional gotos are just test code, right? If
not, the second one is dead.

which looks ok, but after register allocation and prolog/epilogue insertion, I
get this:

   entry (0x8681458):
        .....
        setcc %gr0, %gr1
        goto %disp(label then)
        goto %disp(label else)
        %ar6 = + %ar6, 1
        store %ar6, %gr0
        %ar6 = - %ar6, 1
        %ar6 = + %ar6, 2
        store %ar6, %gr1
        %ar6 = - %ar6, 2

The code after "goto" is disturbing. It looks like spill code, but it's not
going to be ever executed. Any ideas why it's generated? Is there any
additional information I should provide?

Yup, just add the "isTerminator" bit on your gotos in your tablegen
description for the instruction. This informs the code generator that any
spill code has to go above the instructions.

-Chris

Chris Lattner wrote:

> While adding support for branch instructions in my backend, I run into a
> trouble. The code to handle branches looks like:
> The machine code after instruction selection is:
>
> entry (0x8681458):
> %reg1024 = load <fi#-1>
> %reg1025 = load <fi#-2>
> setcc %reg1024, %reg1025
> goto %disp(label then)
> goto %disp(label else)

I assume that the two unconditional gotos are just test code, right? If
not, the second one is dead.

Yes, in the final form there will be "iflt" instruction before the first goto,
making it conditional.

> The code after "goto" is disturbing. It looks like spill code, but it's
> not going to be ever executed. Any ideas why it's generated? Is there any
> additional information I should provide?

Yup, just add the "isTerminator" bit on your gotos in your tablegen
description for the instruction. This informs the code generator that any
spill code has to go above the instructions.

Thanks, this works! I don't yet understand why spill code is needed there at
all, but I'll return to that when I have branches working correctly.

- Volodya

> I assume that the two unconditional gotos are just test code, right? If
> not, the second one is dead.

Yes, in the final form there will be "iflt" instruction before the first goto,
making it conditional.

Ah, ok :slight_smile:

> > The code after "goto" is disturbing. It looks like spill code, but it's
> > not going to be ever executed. Any ideas why it's generated? Is there any
> > additional information I should provide?
>
> Yup, just add the "isTerminator" bit on your gotos in your tablegen
> description for the instruction. This informs the code generator that any
> spill code has to go above the instructions.

Thanks, this works! I don't yet understand why spill code is needed there at
all, but I'll return to that when I have branches working correctly.

I'm not sure either. Can you send the code before and after register
allocation? You might also try -regalloc=linearscan, as the default
allocator is, uhhh, non-optimal.

-Chris

Chris Lattner wrote:

> Thanks, this works! I don't yet understand why spill code is needed there
> at all, but I'll return to that when I have branches working correctly.

I'm not sure either. Can you send the code before and after register
allocation?

Attached.

You might also try -regalloc=linearscan, as the default
allocator is, uhhh, non-optimal.

Ehm.... I get this:

llc: LiveIntervals.cpp:166: virtual bool
llvm::LiveIntervals::runOnMachineFunction(llvm::MachineFunction&): Assertion
`r2iit != r2iMap_.end()' failed.

- Volodya

llc_log (1.14 KB)

Chris Lattner wrote:
> > Thanks, this works! I don't yet understand why spill code is needed there
> > at all, but I'll return to that when I have branches working correctly.
>
> I'm not sure either. Can you send the code before and after register
> allocation?

Attached.

Okay, yeah the spill code looks right. The local allocator can't keep
virtual registers in physical registers across basic blocks. As such, the
vregs are spilled at the end of the entry block and then reloaded in the
blocks they are used.

> You might also try -regalloc=linearscan, as the default
> allocator is, uhhh, non-optimal.

Ehm.... I get this:

llc: LiveIntervals.cpp:166: virtual bool
llvm::LiveIntervals::runOnMachineFunction(llvm::MachineFunction&): Assertion
`r2iit != r2iMap_.end()' failed.

Hrm, that's obviously really bad. Can you send me (offline) the output of
llc with the -debug option set and with this code before the assert:

  std::cerr << "MI: " << i << " " << reg << " "; mii->dump();

Also note that it looks like your tree is a bit out of date. You might
try updating and see if it helps the problem.

-Chris

Chris Lattner wrote:

> > I'm not sure either. Can you send the code before and after register
> > allocation?
>
> Attached.

Okay, yeah the spill code looks right. The local allocator can't keep
virtual registers in physical registers across basic blocks. As such, the
vregs are spilled at the end of the entry block and then reloaded in the
blocks they are used.

Oh, I now understand why you say default allocator is not very efficient.

> Ehm.... I get this:
>
> llc: LiveIntervals.cpp:166: virtual bool
> llvm::LiveIntervals::runOnMachineFunction(llvm::MachineFunction&):
> Assertion `r2iit != r2iMap_.end()' failed.

Hrm, that's obviously really bad. Can you send me (offline) the output of
llc with the -debug option set and with this code before the assert:

  std::cerr << "MI: " << i << " " << reg << " "; mii->dump();

Also note that it looks like your tree is a bit out of date. You might
try updating and see if it helps the problem.

I've updated and rebuild. The bug is still there and the output you've asked
for is attached.

HTH,
Volodya

llc_crash (2.21 KB)

Okay, it looks like a bug in your code generator (though the linscan
allocator should give a better assertion). Specifically, you never define
the %reg1028 register.

-Chris

Chris Lattner wrote: