[PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

Mon Nov 18 07:07:18 PST 2019

djtodoro marked an inline comment as done.
djtodoro added a comment.

@jmorse Thanks for your comments!

> Keeping the main / "backup" locations starts introducing a hierarchy of longevity for locations, which is where I think we want to be heading.

Thanks. I agree. We may think of refactoring this in future by using some different data structures (e.g. lists, arrays or queue) to store all valid location (primary and backups) for a variable, and try to avoid `Backup` kinds for every new desirable location that can occur as backup.

================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:524-526
+      assert((Vars.empty() && EntryValuesBackupVars.empty()) ==
+                 VarLocs.empty() &&
+             "open ranges are inconsistent");
----------------
jmorse wrote:
> djtodoro wrote:
> > jmorse wrote:
> > > Won't having "Vars.empty()" as a clause here mean the assertion fires if the OpenRanges isn't empty, rather than testing whether it's empty?
> > I am not sure any more, since this is not used at the current code..
> I think it should probably read:
> 
>   assert(Vars.empty() == EmptyValuesBackupVars.empty &&
>              Vars.Empty() == VarLocs.empty() &&
>              "open ranges are inconsistent");
>   return VarLocs.empty()
> 
> To ensure that if one is empty, all the others are. Simply asserting that Vars is empty will lead to assertions firing for completely valid states of the pass.
It makes sense. Thanks.

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

https://reviews.llvm.org/D68209