[PATCH] D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging

Scott Linder via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 13 16:57:10 PDT 2020


scott.linder marked an inline comment as done.
scott.linder added inline comments.


================
Comment at: llvm/lib/CodeGen/MIRParser/MILexer.cpp:218-220
+      .Case("nuw", MIToken::kw_nuw)
+      .Case("nsw", MIToken::kw_nsw)
+      .Case("exact", MIToken::kw_exact)
----------------
dblaikie wrote:
> scott.linder wrote:
> > arsenm wrote:
> > > Unrelated change
> > I wish all of this rote mechanical stuff were just automated. Do you have a preference on how I resolve this? The options I'm aware of:
> > 
> > * I can factor out every `clang-format` diff into a separate NFC review, but effectively every non-trivial patch would need one, so I would end up having to manage on the order of twice as many Phabricator reviews.
> > * I can make one pass at the very end to try to pull all the unrelated NFC formatting changes into one commit, but I would rather do this just before committing so I am not constantly having to comb through every patch in the set trying to clean them up.
> > * I can aggressively commit these NFC cleanups as separate patches without reviews as they come up, before posting the actual review.
> > 
> > Or I can lobby on llvm-dev to just lint/format the whole codebase once, and then gate commits on patches not breaking it. This would probably save everyone too much time, and make `git blame` too useful though.
> Most of it is automated or can be - if you're only clang-formatting the lines of code you've edited, it's usually enough - just in this instance happens to be a very long statement & so it gets reformatted together even if you changed unrelated lines in the statement. /maybe/ clang-format could be modified to not do that (to be able to reformat only the edited lines, rather than the whole statement)
> 
> Something like this one - yeah, you could commit that specific formatting (not reformatting the whole file) in advance to ensure the review is simpler/clearer. I think this fits under the general idea of reformatting early if you're making significant changes - like if you're majorly reworking a whole file, it's not uncommon to reformat the whole file. But yeah, if you're touching a statement/block in depth and it's significantly misformatted, easy thing to fix/commit in advance - doesn't need a formal pre-commit review (so long as it's code that's going to be touched/changed soon - don't go reformatting whole files without such motivation, unless they're really egregiously formatted).
Is there any document with the rationale for why LLVM prefers incrementally massaging code into the canonical format, rather than just fixing the entire codebase at once? Like a section in some docs somewhere, or an old llvm-dev thread? It might make the pill easier to swallow if I understand the ideal to which I'm dedicating time fiddling with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76877





More information about the llvm-commits mailing list