[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

In D91734#2432988 <https://reviews.llvm.org/D91734#2432988>, @dblaikie wrote:

> Yeah, it boils down to something like this:
>
>   void f1(const char*, const char*) { }
>   int main() {
>     char prog[1];
>   
>     f1(prog,
>        prog);
>   }
>
> Without the patch, you step to the 'f1' line, and then step into or over the function call (step or next).
> But with these patches applied - you step to the 'f1(' line, then the 'prog);' line, then back to the 'f1(' line, then into/over the function call.
>
> Again, could be acceptable - if those arguments had specific non-trivial code in them (like other function calls) you'd certainly get that step forward/backward behavior - but it's notewhorthy that this is change would make more cases like that & it'd be good to understand why/if that's a good thing overall.

If you dump the IR you should see two GEP instructions, one for each actual parameter expression.  Prior to the patch, FastISel found the second one computed the same value as the first one, and reused the value (in this case, the result of `lea`).  This is one of the 5% of cases where a value is being reused across instructions, with the HEAD compiler. After the patch, we flush the local map after each GEP, so the second one gets its own separate `lea` (with its own source location).  So, you're stepping to the first actual, then to the second, then to the call.  Hope that answers the "why" question.

Whether it's a "good" thing... that path is fraught with peril.  Looking at it somewhat abstractly (as I am sometimes wont to do), Clang is making choices to associate the source location of each parameter with the IR instructions to compute that parameter, and by flushing the map on every IR instruction, LLVM is more faithfully representing that in the final object.  This is -O0 after all, where a more faithful representation is more likely to be more debuggable.  gcc is making entirely different choices, lumping all parameter-prep instructions in with the function call itself.  I'm hard pressed to say one is better than the other; they are different.  Coke and Pepsi; people may have preferences, but that's not the same as being higher on a good-ness scale.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91734/new/

https://reviews.llvm.org/D91734