[PATCH] D78147: [LICM] Try to merge debug locations when sinking

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D78147#1982329 <https://reviews.llvm.org/D78147#1982329>, @vsk wrote:

> Looks good to me with a minor test update. @dblaikie?


Fair enough - still a bit of a preference for not having to use extra storage for this operation, but see how it shakes out with practice I guess.

Test case could probably do with some simplification - and include the original source/repro steps to generate the IR in a comment in the test file. Here's as simple as I could get it at least:

  volatile int a;
  extern int g;
  int g;
  void f1() { 
    while (a) { 
      g = 0;
      if (a)
        g = 0;
    } 
  }

At least it does hit the same line of code, so hopefully it's observable there.

Might be worth explaining validating that the specific "scope" for the chosen DILocation is correct - showing that it picked the nearest enclosing scope of the two stores. (this is existing functionality in the merge location - but showing the N-way merge preserves this too) & maybe even generalizing the test to make sure it exercises the N-way merge (ensuring the size of the vector's at least 3 for this test case? probably by adding another "if (a) g = 0;" maybe? or something like that)


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

https://reviews.llvm.org/D78147