[PATCH] D87272: [lld] Buffer writes when composing a single diagnostic

Sun Oct 4 23:46:50 PDT 2020

MaskRay added a comment.

In D87272#2311061 <https://reviews.llvm.org/D87272#2311061>, @dblaikie wrote:

> In D87272#2310448 <https://reviews.llvm.org/D87272#2310448>, @MaskRay wrote:
>
>> In D87272#2295704 <https://reviews.llvm.org/D87272#2295704>, @dblaikie wrote:
>>
>>> Not sure I follow - the patch description seems to be quite focussed on the performance implications of this change - it sounds like, reading the patch description/commit message that that is the motivation.
>>>
>>> Oh, perhaps I see another reason that might be less nuanced: This will probably break color printing on Windows when using the normal Windows color API - because it's a stateful API, not something that can be encoded in the stream: https://github.com/llvm/llvm-project/blob/d3f6972abb9c0ac06ddabf61697754c3c6f5d11b/llvm/lib/Support/raw_ostream.cpp#L513
>>
>> I find another reason, probably still weak: I want to check a large binary with many out of range relocations. lld will emit a large number of warnings (if --noinhibit-exec). This patch can make it faster.
>
> Could the uninteresting warnings be disabled? if all the warnings are interesting, I'd guess a bunch of processing would need to be done to make it actionable - or a lot of human time to read through them all, either way that time probably dwarfs the extra time lld takes to print those errors.

No. They were errors but downgraded to warnings.

> In any case - I think colour can't be portably buffered (well, in addition to the existing buffering in the ostream itself) so far as I can see in the implementation. Does that understanding seem right? Is there a way that could be addressed?

The code makes messages line buffered, instead of full buffered. (Not to say that buffered ANSI escape sequences work as well). The currently mere downside of this patch is the introduction of `reportDiagnostic` (added complexity). There are a number of small advantage on the up side, though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87272