[PATCH] Fixing line numbers of inlined allocas

Wolfgang Pieb wolfgang_pieb at playstation.sony.com
Fri Oct 17 11:53:19 PDT 2014


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:813
@@ -812,1 +812,3 @@
          BI != BE; ++BI) {
+      // Don't update static allocas, as they may get moved later.
+      if (isa<AllocaInst>(BI) &&
----------------
dblaikie wrote:
> To avoid isa + cast, we'd probably write this as:
> 
>   if (auto *AI = dyn_cast<AllocaInst>(BI))
>     if (isa<Constant>(AI->getArraySize()))
>       continue;
> 
> Also, given that static allocas are so special, is there not a more direct way to test for them than isa<Constant> on the array size?
Regarding the static allocas, there is AllocaInst::isStaticAlloca() but it's not usable here because it tests for the instruction being part of the entry block, which the freshly inlined alloca is not (yet). Otherwise, isStaticAlloca also just tests for a constant array size.

Thanks for the pointer on the dyn_cast, I'll make that change.

Actually, I'm thinking now that my change is in the wrong place. It should be inside the if (DL.isUnkown()) branch. We only want to skip allocas that have no debug information to begin with, not the ones that do.

================
Comment at: test/DebugInfo/debug-info-always-inline.ll:7
@@ +6,3 @@
+; /* BEGIN SOURCE */
+; int __attribute__((always_inline)) foo()
+; {
----------------
dblaikie wrote:
> Should we add nodebug to this function just to demonstrate why the other solutions we discussed were insufficient? (eg: we can't describe it as inlining, etc)
> 
> One day we might want to describe it as inlining in the non-nodebug case (and update the prologue handling code to cope with this), but we'd still need to do this for nodebug.
> 
> Or test both & describe that one /could/ change but mention the prologue complications.
> 
> Or just write a comment describing the gotchas here.
I think nodebug is handled by the front end, so the difference in the IR is simply that the individual instructions have no debug information. There does not seem to be any distinction on the function level.

I'll describe the problems with the allocas/prologue in more detail. There is the more general question of what to do with inlined instructions without debug info, but that seems beyond the scope of this patch. Do you want me to add the gist of the discussion to the test case?

http://reviews.llvm.org/D5401






More information about the llvm-commits mailing list