PHINode containing itself causes segfault when compiling Blender OpenCL kernel with R600 backend

Hi!

I'm trying to run Blender using Mesa OpenCL implementation on a radeonsi card. First the kernel didn't want to compile, but that was caused by a bug in it (they were using . instead of -> in 1 place), and after fixing this bug I've got the kernel to compile...

...But after that, LLVM started to crash during translation of IR into shader code with R600 backend.

I've done some investigation and figured out that the crash is caused by a PHINode containing itself. SIAnnotateControlFlow::handleLoopCondition() can't handle such situation - it recurses into itself, calls Phi->eraseFromParent() inside the inner execution, returns into outer one, gets zeroed out object and crashes when trying to do something with its members... for example when trying to erase it again.

I have no real background in LLVM or GCC, so the concept of PHINode itself was a real discovery for me :slight_smile: and PHINode containing itself does look even more strange... I've tried to understand the semantics of such PHINodes from reading the code and got a suspicion that the rest of LLVM code just ignores PHINodes equal to their parent... So I've tried to fix the bug by making handleLoopCondition() skip IncomingValues equal to the Phi itself, but the bug didn't go away! Surprisingly, PHINode may not just contain itself directly, but it also may contain itself inside another PHINode, i.e. Phi->getIncomingValue(0)->getIncomingValue(0) == Phi, which results in the same problem with SIAnnotateControlFlow...

Besides "how to make a correct fix" :), my question also is: what are the real semantics of a PHINode containing itself directly or indirectly? I've done some tracing and saw such PHINodes added by the optimizer, in llvm::InlineFunction()... but what do they mean and how to deal with them correctly?

Hi!

I'm trying to run Blender using Mesa OpenCL implementation on a
radeonsi card. First the kernel didn't want to compile, but that was
caused by a bug in it (they were using . instead of -> in 1 place),
and after fixing this bug I've got the kernel to compile...

...But after that, LLVM started to crash during translation of IR
into shader code with R600 backend.

Can you file a bug for this at bugs.freedesktop.org and post the output
of blender with the environment variable R600_DEUBG=cs,compute

-Tom

Can you file a bug for this at bugs.freedesktop.org and post the output
of blender with the environment variable R600_DEUBG=cs,compute

OK, I'll do that soon (in several hours). I've looked at that output by myself, the blender kernel llvm assembly dump was there. I can also provide the stack traces of the crash...

But can you give me a hint about the PHINode? What does it mean when it contains itself?..

If a phi-node contains itself as one of its inputs, then that input will contain the last dynamic value of that phi-node (which means, essentially, that its value does not change). If you have:

r1 = phi (bb1, p1) (bb2, r1)

... then if control-flow enters from basic block bb1, then r1 is set to the value p1 whereas if control-flow enters from basic block bb2 r1 will have its value set to r1 (meaning that it won't change).

For more information on phi-nodes and why they exist, I recommend reading "Efficiently computing static single assignment form and the control dependence graph" by Ron Cytron et. al.

Regards,

John Criswell

Thanks for the explanation… but does it satisfy SSA? It seems that register gets overwritten more than 1 time when using itself as input? Shouldn’t SSA unroll such structures into ones that don’t reference itself?

And is it the same with LLVM’s internal structures (PHINode)? I mean, is PHINode a “register” or just an “expression”? If it’s meant to be an expression, then it still looks strange to be recursive, because there’s no “previous” value for it to reference…

Can you file a bug for this at bugs.freedesktop.org and post the output
of blender with the environment variable R600_DEUBG=cs,compute

Filed https://bugs.freedesktop.org/show_bug.cgi?id=84232

Thanks for the explanation... but does it satisfy SSA? It seems that register gets overwritten more than 1 time when using itself as input? Shouldn't SSA unroll such structures into ones that don't reference itself?

SSA is short for Single *Static* Assignment. SSA means that there is only one instruction that sets the value of a variable. That instruction can be executed multiple times (e.g., it can be in a loop or in a function that is called multiple times), but when you look at the program statically, the variable is only defined once.

So, to answer your question, yes, the PHINode does satisfy the SSA requirements.

And is it the same with LLVM's internal structures (PHINode)? I mean, is PHINode a "register" or just an "expression"? If it's meant to be an expression, then it still looks strange to be recursive, because there's no "previous" value for it to reference...

As with all other LLVM instructions, the instruction ("expression" as you call it) and the virtual register it defines are one and the same.

I know it's a bit of a dense read, but you should really look at the paper I mentioned in the last email. That paper explains what SSA is and how to put code into SSA form. It also describes the phi-node in some detail.

Regards,

John Criswell

Can you file a bug for this at bugs.freedesktop.org and post the output
of blender with the environment variable R600_DEUBG=cs,compute

Can you give me a hint on what handleLoop() and handleLoopCondition() functions in SIAnnotateControlFlow are intended for?