Bug 84232

Created attachment 106718 [details]
Stack trace

Created attachment 107527 [details]
Attempt to reduce testcase using bugpoint

Tried to reduce the case using bugpoint:

PATH=/usr/lib/llvm-3.5/bin/:$PATH bugpoint-3.5 kernel.ll --llc-safe --tool-args -- -march=r600 -mcpu=oland

The result leads to other crash, maybe it's also a bug, but it's definitely another bug:

llc-3.5 -march=r600 -mcpu=oland bugpoint-reduced-simplified.ll

Which leads to a segfault on a non-assert build and to the following message on a release+asserts build:

Unhandled loop condition!
UNREACHABLE executed at /var/home/vitali/setup/llvm-toolchain-3.5-3.5/lib/Target/R600/SIAnnotateControlFlow.cpp:259!

This is probably related to bugpoint replacing the branch conditions with constants, which makes conditional branches effectively unconditional :) and SIAnnotateControlFlow::handleLoopCondition also doesn't handle this case.

Looks like an easy to fix problem?

(In reply to Vitaliy Filippov from comment #2)

> 
> Unhandled loop condition!
> UNREACHABLE executed at
> /var/home/vitali/setup/llvm-toolchain-3.5-3.5/lib/Target/R600/
> SIAnnotateControlFlow.cpp:259!
> 
> This is probably related to bugpoint replacing the branch conditions with
> constants, which makes conditional branches effectively unconditional :) and
> SIAnnotateControlFlow::handleLoopCondition also doesn't handle this case.
> 
> Looks like an easy to fix problem?

The StructurizeCFG pass can actually create branches on constants, although it shouldn't, which I've seen several times before. I've had a partial fix for SIAnnotateControlFlow handling branches on constants for a while, but I've been putting off fixing it until the less noticeable StructurizeCFG problem is fixed.

The problem is that the inability of SIAnnotateControlFlow to handle constants confuses bugpoint and it doesn't emit the correct testcase for the initial crash which I've reported in this bug...

OK if I add you to the CC list? :)

It looks like the following sequence may trigger this bug:

for () {
  if (){
    break;
  }
  if () {
    if () {
        break;
    }
  }
  if () {
     break;
  }
}

I will investigate further.

Hm, I've tried to reproduce your testcase and got another bug:

int main()
{
  int i;
  for (i = 0; i < 100; i++)
  {
    if (!(i%10))
    {
      i = i%50;
      if (i%2 == 0)
        break;
    }
  }
  return i;
}

clang-3.5 -S -emit-llvm test.c
llc-3.5 -march=r600 -mcpu=oland test.ll 

>>>>

Program received signal SIGSEGV, Segmentation fault.
llvm::BasicBlock::getTerminator (this=0x471f08) at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/IR/BasicBlock.cpp:126
126       return dyn_cast<TerminatorInst>(&InstList.back());
(gdb) bt
#0  llvm::BasicBlock::getTerminator (this=0x471f08) at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/IR/BasicBlock.cpp:126
#1  0x00007ffff7442c9a in (anonymous namespace)::SIAnnotateControlFlow::handleLoopCondition (this=this@entry=0x44c960, Cond=0x471eb0, Broken=Broken@entry=
    0x486a50) at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/Target/R600/SIAnnotateControlFlow.cpp:271
#2  0x00007ffff7443d6c in handleLoop (Term=0x486af8, this=0x44c960)
    at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/Target/R600/SIAnnotateControlFlow.cpp:288
#3  (anonymous namespace)::SIAnnotateControlFlow::runOnFunction (this=0x44c960, F=...)
    at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/Target/R600/SIAnnotateControlFlow.cpp:325
#4  0x00007ffff662a3d7 in llvm::FPPassManager::runOnFunction (this=0x43e930, F=...)
    at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/IR/LegacyPassManager.cpp:1545
#5  0x00007ffff662a45b in llvm::FPPassManager::runOnModule (this=0x43e930, M=...)
    at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/IR/LegacyPassManager.cpp:1565
#6  0x00007ffff662ce19 in runOnModule (M=..., this=0x431140) at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/IR/LegacyPassManager.cpp:1623
#7  llvm::legacy::PassManagerImpl::run (this=0x430e50, M=...) at /var/home/vitali/setup/llvm/llvm-toolchain-3.5-3.5/lib/IR/LegacyPassManager.cpp:1730
#8  0x0000000000411a11 in ?? ()
#9  0x000000000040cd98 in main ()

Another thought: it seems PHINode that contain itself are also a product of StructurizeCFG! After running `opt-3.5 -S -structurizecfg kernel.ll > kernel-scfg.ll` on the kernel from attached blender output, kernel-scfg.ll contains phinode that reference themselves directly... While kernel.ll did not contain such PHINodes.

grep -P '(%\S+) = phi.*\1' kernel-scfg.ll 
  %18919 = phi float [ %18919, %Flow196 ], [ %19044, %Flow162 ], [ %18839, %18838 ]
  %18920 = phi float [ %18920, %Flow196 ], [ %18915, %Flow162 ], [ %18840, %18838 ]
  %18921 = phi <4 x float> addrspace(1)* [ %18921, %Flow196 ], [ %18896, %Flow162 ], [ %18841, %18838 ]
  %18922 = phi i32 [ %18922, %Flow196 ], [ %18895, %Flow162 ], [ %18842, %18838 ]
  %18923 = phi i32 [ %18923, %Flow196 ], [ %18894, %Flow162 ], [ %18843, %18838 ]
  %19208 = phi i64 [ %19208, %Flow195 ], [ %indvars.iv.next.i.i, %Flow164 ], [ %18953, %Flow161 ]
  %19209 = phi float [ %19209, %Flow195 ], [ %19056, %Flow164 ], [ %18954, %Flow161 ]
  %19210 = phi float [ %19210, %Flow195 ], [ %19055, %Flow164 ], [ %18955, %Flow161 ]
  %19211 = phi float [ %19211, %Flow195 ], [ %19054, %Flow164 ], [ %18956, %Flow161 ]
  %19212 = phi i32 [ %19212, %Flow195 ], [ %19053, %Flow164 ], [ %18957, %Flow161 ]
  %19213 = phi i32 [ %19213, %Flow195 ], [ %19052, %Flow164 ], [ %18958, %Flow161 ]
  %19227 = phi i1 [ %19227, %Flow195 ], [ %19059, %Flow164 ], [ true, %Flow161 ]
  %19590 = phi <4 x float> [ %19590, %Flow188 ], [ %20245, %Flow171 ], [ %19529, %Flow169 ]
  %19591 = phi <4 x float> [ %19591, %Flow188 ], [ %20246, %Flow171 ], [ %19530, %Flow169 ]
  %19592 = phi float [ %19592, %Flow188 ], [ %20247, %Flow171 ], [ %19531, %Flow169 ]
  %19593 = phi <4 x float> [ %19593, %Flow188 ], [ %20248, %Flow171 ], [ %19532, %Flow169 ]
  %19594 = phi <4 x float> [ %19594, %Flow188 ], [ %20249, %Flow171 ], [ %19533, %Flow169 ]
  %19595 = phi <4 x float> [ %19595, %Flow188 ], [ %20250, %Flow171 ], [ %19534, %Flow169 ]
  %19596 = phi <4 x float> [ %19596, %Flow188 ], [ %20251, %Flow171 ], [ %19535, %Flow169 ]
  %19597 = phi i32 [ %19597, %Flow188 ], [ %20252, %Flow171 ], [ %19536, %Flow169 ]
  %19598 = phi i32 [ %19598, %Flow188 ], [ %20253, %Flow171 ], [ %19537, %Flow169 ]
  %19599 = phi i32 [ %19599, %Flow188 ], [ %20254, %Flow171 ], [ %19538, %Flow169 ]
  %19600 = phi <4 x float> [ %19600, %Flow188 ], [ %20255, %Flow171 ], [ %19539, %Flow169 ]
  %19601 = phi <4 x float> [ %19601, %Flow188 ], [ %20256, %Flow171 ], [ %19540, %Flow169 ]
  %19609 = phi float [ %19609, %Flow188 ], [ undef, %Flow171 ], [ %19548, %Flow169 ]
  %19610 = phi float [ %19610, %Flow188 ], [ undef, %Flow171 ], [ %19549, %Flow169 ]
  %19611 = phi float [ %19611, %Flow188 ], [ undef, %Flow171 ], [ %19550, %Flow169 ]
  %19612 = phi i32 [ %19612, %Flow188 ], [ undef, %Flow171 ], [ %19551, %Flow169 ]
  %19613 = phi i32 [ %19613, %Flow188 ], [ undef, %Flow171 ], [ %19552, %Flow169 ]
  %19625 = phi i1 [ %19625, %Flow188 ], [ %20257, %Flow171 ], [ %19569, %Flow169 ]
  %19627 = phi i1 [ %19627, %Flow188 ], [ %20258, %Flow171 ], [ %19571, %Flow169 ]

Created attachment 107681 [details]
Reduced Test Case

Do I understand it right that the "Reduced test case" is about constant branch condition?

I've managed to make a reduced test case with a recursive PHI after StructurizeCFG !!!

int main()
{
  int i, j = 0;
  while (j % 2)
  {
    if (j > 100)
      j += 3;
    else if (j > 50)
      break;
  }
  return j;
}

clang-3.5 -S -emit-llvm test3.c
opt-3.5 -S -structurizecfg test3.ll > test3cfg.ll

Simple to visualise with graphviz:

( echo 'digraph G {'; perl -ne 'if (/\%(\S+) = phi/) { my $a = $1; print "$a -> $_;\n" for /\[ \%([^\s,]+)/g; }' test3cfg.ll; echo '}' ) > test3cfg.dot
dot -Tsvg test3cfg.dot > test3cfg.svg

%21 and %26 phi's loop in the result.

Also there is a branch on constant in the end of test3cfg.ll:

br i1 false, label %14, label %Flow2

Created attachment 108266 [details]
StructurizeCFG IR output for the sample from comment 10

Hm... Checked the test from comment 6, the result is the same - there are two PHINode loops and one branch on constant.

Also checked the code, it looks like crash from comment 6 is also triggered by a branch-on-constant, the difference is that it crashes when a ConstantInt is dyn_cast<>'ed to incorrect class - Instruction (ConstantInt isn't an Instruction!!) in handleLoopCondition() and getTerminator() is called on it.

It seems that the StructurizeCFG branch-on-constant bug not only crashes R600, but actually generates invalid code!

It's clearly seen that in the IR code from comment 11 (structurizecfg output of comment 10) (j > 50) comparison is contained in block %14, so it must be reachable. But it's unreachable because the only branch there is that constant branch instruction from %25 that always avoids %14...

Wow!! Tom, did you just fix this bug with commit 857550322c6fe679d17d35c885606ae1d8cf43b6?

(In reply to Vitaliy Filippov from comment #14)
> Wow!! Tom, did you just fix this bug with commit
> 857550322c6fe679d17d35c885606ae1d8cf43b6?

Possibly, does blender work now (or at least does it no longer crash)?

(In reply to Tom Stellard from comment #15)
> Possibly, does blender work now (or at least does it no longer crash)?

Yes, it doesn't crash anymore, thanks!

There are still other bugs that prevent it from running, and as I understand most renderer features are disabled with default flags/defines under Mesa... From https://developer.blender.org/T41912 - "Note though: Building without SVM gives you only very basic clay like rendering, not really usable."

The first bug is about libclc - it incorrectly references fabsf() in R600 bytecode files, while it should probably use llvm.fabs.f32()... Where to report it?

Second, it seems Blender developers are using half-precision floats (vstore_half4 function)... it's used in only one place and I don't know if its usage is really critical, but it of course prevents the kernel from compiling... I've reported it here https://developer.blender.org/T42813

Vitaliy, is this still an issue?

Original bug was fixed as per comment 14 and comment 16. Please open new bug reports for any further issues. Closing as fixed.