[PATCH] D73534: [WIP][DebugInfo] Enable the debug entry values feature by default

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 28 11:34:47 PST 2020


dblaikie added a comment.

Do you have size measurements for the impact of this change (on, say, a self-host optimized build of clang)?

& I think it's probably best to just change the default value of the flag - rather than changing the default and removing the flag. Giving folks some time with an escape hatch might be important.

Is this patch intended for commit - or as a preview/working area? It seems it's got a bunch of things that could/should be done in independent commits (like the "Fix the assertions regarding updating the CallSiteInfo from the MF" parts, for instance - perhaps that was what was raised by another reviewer earlier).



================
Comment at: llvm/include/llvm/Target/TargetMachine.h:240-242
+  void setSupportsDebugEntryValues(bool Enable) {
+    Options.SupportsDebugEntryValues = Enable;
+  }
----------------
Why is this a target machine feature? What aspect of debug entry values is target specific? Is there something about their architectures that makes it unrepresentable? (that seems drastic given the generality of the feature)

I'm just a bit hesitant to introduce target divergence if it can reasonably be avoided - it'll make debug info comparisons, cross platform work, etc, more difficult.


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

https://reviews.llvm.org/D73534





More information about the llvm-commits mailing list