[PATCH] D45593: [DebugInfo][OPT] Fixing a couple of DI duplication bugs of CloneModule

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/trunk/lib/IR/DebugInfo.cpp:141
+    else
+      llvm_unreachable("unexpected imported entity type");
+  }
----------------
rtereshin wrote:
> aprantl wrote:
> > aprantl wrote:
> > > aprantl wrote:
> > > > rtereshin wrote:
> > > > > alberto_magni wrote:
> > > > > > This commit is causing  failures in my SPEC2006 tests (xalancbmk among others) with -flto=thin and -g.
> > > > > > The unreachable in this line gets triggered. 
> > > > > > 
> > > > > > This is because the if-cascade above does not handle the case for DIGlobalVariable.
> > > > > > Probably the code that handles GlobalVariables at the top of the function needs to be replicated in one if-case here too.
> > > > > Hi @alberto_magni,
> > > > > 
> > > > > Is there a chance you could extract a regression test from one of those SPEC2006 tests?
> > > > > 
> > > > > Thanks!
> > > > > Roman
> > > > If it's just a DIGlobalVariable appearing in that list, you should be able to construct one manually without much trouble.
> > > ```
> > > $ cat /tmp/using.cpp
> > > namespace s {
> > >   int i;
> > > }
> > > 
> > > using s::i;
> > > 
> > > int f() { return i; }
> > > ```
> > > 
> > > Using declarations are represented as imported entities.
> > That also means that we'll need to accept pretty much anything in that list :-(
> Thanks, I'll try that!
> 
> So, we gotta remove the unreachable, I suppose?
Yes, but perhaps think about what else might appear in a using statement that we would need to process here and also whether it can be dangerous to process something twice, because it is reachable via the imported entities and some other means (as in my example above) so we don't end up with duplicates / infinite loops / etc.


Repository:
  rL LLVM

https://reviews.llvm.org/D45593