[PATCH] D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 13 15:51:32 PDT 2020


dblaikie 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)
----------------
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).


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