[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