[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: lib/CodeGen/CodeGenFunction.h:217
+  /// statements.
+  llvm::SmallVector<bool, 4> LabelSeenStack;
+
----------------
rjmccall wrote:
> Shouldn't this be maintained by some existing scoping structure like LexicalScope?
I think I need a structure that gets pushed and popped when we enter and exit a compound statement, but I couldn't find one in CodeGenFunction.

Adding a flag to LexicalScope would fail to disable lifetime markers in some cases. For example, in the code below (which I think is valid), the lifetime markers of "i" wouldn't get disabled because a LexicalScope is pushed at the beginning of CodeGenFunction::EmitForStmt:

```
void foo2(int *);
int c;

void foo1(int n) {
  int *p = 0;

label1:
  foo2(p);
  for (int i = 0; i < n; ++i)
    if (i == 5)
      p = &i;

  if (c)
    goto label1;
}
```

If we want to avoid introducing new structures to track labels, perhaps we can just check whether a label was seen so far in the current function rather than the current compound statement, but I'm not sure how much impact that would have on performance.


================
Comment at: lib/CodeGen/CodeGenFunction.h:236
+    LabelSeenStack.back() = true;
+  }
+
----------------
rjmccall wrote:
> A label in a nested scope acts like a label in outer scopes, too.
I think you are suggesting setting all the flags on the stack rather than just the top (or maybe not?), but I'm not sure why that's necessary. Could you elaborate on your comment a little more and show me a simple example when this code would fail to disable lifetime markers when it should?

The boolean flag set to true is propagated to the outer scope's flag when we have completed processing the compound statement and LabelSeenRAII's destructor is called. So, for example, if I enclose label1 in function jump_backward_over_declaration of the test case, 
IRGen disables lifetime markers for "i" even though "i" is declared in the outer compound statement of label1.


https://reviews.llvm.org/D27680