aprantl added inline comments.
================
Comment at: lib/CodeGen/CGExpr.cpp:72
+ // Set debug location in order to preserve the scope
+ Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
if (AllocaAddr)
----------------
vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > aprantl wrote:
> > > > vsk wrote:
> > > > > I think we need to be a bit more careful here. The current debug
> > > > > location stored in the builder may not be an artificial 0-location.
> > > > > This may cause non-linear single-stepping behavior. Consider this
> > > > > example:
> > > > >
> > > > > ```
> > > > > void foo() {
> > > > > bar();
> > > > > if (...) {
> > > > > int var = ...; //< Clang emits an alloca for "var".
> > > > > }
> > > > > ...
> > > > > ```
> > > > >
> > > > > The current debug location at the line "int var = ..." would be at
> > > > > line 4. But the alloca is emitted in the entry block of the function.
> > > > > In the debugger, this may result in strange single-stepping behavior
> > > > > when stepping into foo(). You could step to line 4, then line 2, then
> > > > > line 3, then line 4 again.
> > > > >
> > > > > I think we can avoid that by setting an artificial location on
> > > > > allocas.
> > > > > I think we can avoid that by setting an artificial location on
> > > > > allocas.
> > > > An alloca doesn't really generate any code (or rather.. the code it
> > > > generates is in the function prologue). In what situation would the
> > > > debug location on an alloca influence stepping? Are you thinking about
> > > > the alloca() function?
> > > An alloca instruction can lower to a subtraction (off the stack pointer)
> > > though: https://godbolt.org/g/U4nGzJ.
> > >
> > > `dwarfdump` shows that this subtraction instruction is actually assigned
> > > a location -- it currently happens to be the first location in the body
> > > of the function. I thought that assigning an artificial location to the
> > > alloca would be a first step towards fixing this.
> > >
> > > Also, using an artificial location could mitigate possible bad
> > > interactions between code motion passes and IRBuilder. If, say, we were
> > > to set the insertion point right after an alloca, we might infer some
> > > arbitrary debug location. So long as this inference happens, it seems
> > > safer to infer an artificial location.
> > >
> > >
> > This may have unintended side-effects: By assigning a debug location to an
> > alloca you are moving the end of the function prolog to before the alloca
> > instructions, since LLVM computes the end of the function prologue as the
> > first instruction with a non-empty debug location. Moving the end of the
> > function prologue to before that stack pointer is adjusted is wrong, since
> > that's the whole point of the prologue_end marker.
> >
> > To me it looks more like a bug in a much later stage. With the exception of
> > shrink-wrapped code, the prologue_end should always be after the stack
> > pointer adjustment, I think.
> Thanks for explaining, I didn't realize that's how the end of the function
> prologue is computed! Should we leave out the any debug location changes for
> allocas in this patch, then?
I think that would be better. You might want to double-check
PrologueEpilogueInserter and how the FrameSetup attribute is attached to
MachineInstrs in case my knowledge is out-of-date.
Repository:
rC Clang
https://reviews.llvm.org/D47097
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
