Fix pruned jump table basic blocks causing compilation failure (Bug 28102)

[madscientist159]

Summary

Ignore unreachable basic blocks that remain in the jump table list(s) after basic block pruning has completed. This fixes Bug 28102.

Notes:
The issues described in bug 28102 were traced to unreachable BB pruning not deleting the associated BB from the jump tables. After several false starts it doesn't appear possible and/or a good idea to remove the BB from the jump table list during pruning, so I ended up modifying the assembler output to ignore unreachable BBs that remain in the jump table lists after pruning.

Please let me know if this is the right approach or if I should be doing this some other way.

[hfinkel]

Looks reasonable to me.

Do you have a small test case? If not, and you don't know how to use bugpoint to reduce your current test case, I'll be happy to provide instructions.

[madscientist159]

In D22435#486173, @hfinkel wrote:

Looks reasonable to me.

Do you have a small test case? If not, and you don't know how to use bugpoint to reduce your current test case, I'll be happy to provide instructions.

I've tried a number of ways to create a test case but thus far have had no success. Splitting clang's compile run into emit-llvm and llc does not reproduce the issue, making bugpoint mostly useless.

The general code structure that triggers the fault is here:
https://llvm.org/bugs/attachment.cgi?id=16686

However, this also needs to be part of a larger source file to recreate the problem.

[majnemer]

Hmm. I'm not terribly convinced this is the right fix and none of the attachments in PR28102 seem to compile.

While I don't doubt that this patch resolves a problem for you, I worry that it is symptomatic of a bigger problem. For example, it might also indicate that the CFG is faulty.

[madscientist159]

In D22435#486277, @majnemer wrote:

Hmm. I'm not terribly convinced this is the right fix and none of the attachments in PR28102 seem to compile.

I am unable to post the actual codebase triggering the issue due to the license we received the codebase under. As such, I don't think I am going to be able to provide a self-contained, compilable test case for this issue.

Does this mean, in effect, that we have to wait for one of the larger open source codebases to trigger the bug before it can be fixed?

[nemanjai]

Hi Timothy,
regarding Clang + LLC not reproducing the issue...

It is important that you produce the .ll file with all the information it would have in a "straight-through" compilation. That is, you need to pass the same optimization option to Clang when producing the .ll file. This would of course perform all the IR -> IR transformations which may mask your problem, so you need to pass -mllvm -disable-llvm-optzns to Clang.

So based on the information you attached to PR28102, it would be something like:
-fPIC -Wno-switch -Wno-unused-value -fvisibility=hidden -DNDEBUG -g -O2 -std=c++11 -mllvm -disable-llvm-optzns -S -emit-llvm

Hopefully, this will help with bugpoint and hopefully bugpoint will reduce/obscure the code enough that IP is no longer an issue.

[madscientist159]

In D22435#486775, @nemanjai wrote:

Hi Timothy,
regarding Clang + LLC not reproducing the issue...

It is important that you produce the .ll file with all the information it would have in a "straight-through" compilation. That is, you need to pass the same optimization option to Clang when producing the .ll file. This would of course perform all the IR -> IR transformations which may mask your problem, so you need to pass -mllvm -disable-llvm-optzns to Clang.

So based on the information you attached to PR28102, it would be something like:
-fPIC -Wno-switch -Wno-unused-value -fvisibility=hidden -DNDEBUG -g -O2 -std=c++11 -mllvm -disable-llvm-optzns -S -emit-llvm

Hopefully, this will help with bugpoint and hopefully bugpoint will reduce/obscure the code enough that IP is no longer an issue.

Unfortunately this still doesn't work. Compiling directly with clang yields:
fatal error: error in backend: Undefined temporary symbol (technically this is the assembler choking on the invalid label)

Running clang -emit-llvm and llc separately produces an assembler file that does not contain the invalid label. Very odd.

[nemanjai]

Hi Timothy,
I know we've kind of left this behind since we don't have a way to investigate it without a test case that reproduces the problem. However, I'm wondering if this is still a problem. If so, is there any way we can try to reproduce it and try to find the underlying problem?

[lei]

PR39124 has a reduced IR that seem to be the same issue this fixes. However I think Chri's suggested fix in PR28102 to remove the MBB references from the jump tables is a better way to go. Can you please give that a try?

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

for (unsigned JTI = 0, e = JT.size(); JTI != e; ++JTI) {
     // second label is actually referenced by the code.
     if (JTInDiffSection && DL.hasLinkerPrivateGlobalPrefix())
       // FIXME: This doesn't have to have any specific name, just any randomly
       // named and numbered 'l' label would work.  Simplify GetJTISymbol.
       OutStreamer->EmitLabel(GetJTISymbol(JTI, true));
 
     OutStreamer->EmitLabel(GetJTISymbol(JTI));
 
-    for (unsigned ii = 0, ee = JTBBs.size(); ii != ee; ++ii)
+    for (unsigned ii = 0, ee = JTBBs.size(); ii != ee; ++ii) {
+      // If a JTBB has a number of -1 it was pruned as unreachable during
+      // optimization.  It is not feasible to remove the BB from the JT
+      // table during pruning, so simply ignore unreachable JTBBs here.
+      if (JTBBs[ii]->getNumber() == -1)
+        continue;
       EmitJumpTableEntry(MJTI, JTBBs[ii], JTI);
     }
+  }
   if (!JTInDiffSection)
     OutStreamer->EmitDataRegion(MCDR_DataRegionEnd);
 }
 
 /// EmitJumpTableEntry - Emit a jump table entry for the specified MBB to the
 /// current stream.
 void AsmPrinter::EmitJumpTableEntry(const MachineJumpTableInfo *MJTI,
                                     const MachineBasicBlock *MBB,