Fix inconsistency with/without debug information (-g)

[jeroen.dobbelaere]

Summary

This fixes an inconsistency in code generation when compiling with or without debug information (-g).
When debug information is available in an empty block, the original test would fail, resulting in possibly different code.

[JDevlieghere]

Hi Jeroen, any chance you could add a small test to ensure we'd notice if this regresses in the future?

[jeroen.dobbelaere]

In D49467#1166203, @JDevlieghere wrote:

Hi Jeroen, any chance you could add a small test to ensure we'd notice if this regresses in the future?

Yes, I am working on it.

[jeroen.dobbelaere]

• Updated existing (related) testcase to actually test something
• Added new testcase showing the problem

[probinson]

Please rename the new test to simply 'codegenprepare-and-debug.ll` (the convention of a date on the test name is obsolete, we don't do that for new tests anymore).

llvm/test/Other/2018-07-20-codegenprepare-and-debug.ll

3 ↗

(On Diff #156463)

It would be good to be explicit about the X86 dependency:
; REQUIRES: x86-registered-target

4 ↗

(On Diff #156463)

it's should be its here.

[jeroen.dobbelaere]

Updated.

[JDevlieghere]

Looks good to me, but I'll defer to Paul

[probinson]

It seems like the metadata could be reduced further. In particular, do you really need all those different types? The dbg.value needs only !11, I think you should be able to remove !12 through !18 (and references to them).

[jeroen.dobbelaere]

Reduced the debug information.

I am not sure it is minimal now, but when I tried removing more, I started getting 'invalid debug info' messages.

[jeroen.dobbelaere]

Found one extra line of debug info that could be removed.

[JDevlieghere]

Please run the test through llvm-as | llvm-dis, this will fix the numbering of the DI nodes.

[jeroen.dobbelaere]

Reassembled to reset the debug id's

[probinson]

In D49467#1176829, @jeroen.dobbelaere wrote:

Reassembled to reset the debug id's

Oh nice! I wasn't aware of that trick.

The other thing we like in debug-info tests, is to reduce the function attributes as much as possible. I suspect you can eliminate attributes #1 and instead have @foobar use #0. Do that and LGTM.

[jeroen.dobbelaere]

Reduced function attributes.

[jeroen.dobbelaere]

! In D49467#1177034, @probinson wrote:
Oh nice! I wasn't aware of that trick.

You just need to remember to take a copy of the 'comments' in your file and to copy those manually. Otherwise, the test driver is gone (as I noticed the hard way).

[jeroen.dobbelaere]

Jonas, Paul,

could one of you commit this change ?

Thanks !

Jeroen Dobbelaere

llvm/lib/CodeGen/CodeGenPrepare.cpp

         isa<IndirectBrInst>(Pred->getTerminator())))
     return true;
 
-  if (BB->getTerminator() != BB->getFirstNonPHI())
+  if (BB->getTerminator() != BB->getFirstNonPHIOrDbg())
     return true;
 
   // We use a simple cost heuristic which determine skipping merging is