[PATCH] D113030: Add a new tool for parallel safe bisection, "llvm-bisectd".

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D113030#3125984 <https://reviews.llvm.org/D113030#3125984>, @aemerson wrote:

> In D113030#3109509 <https://reviews.llvm.org/D113030#3109509>, @dblaikie wrote:
>
>> In D113030#3103971 <https://reviews.llvm.org/D113030#3103971>, @aemerson wrote:
>>
>>> In D113030#3103903 <https://reviews.llvm.org/D113030#3103903>, @lebedev.ri wrote:
>>>
>>>> I would like to note that there's some rudimentary support
>>>> for previous generation of this scattered throught the codebase,
>>>> namely DebugCounters for bugpoint.
>>>>
>>>> I don't really have an opinion on the proposal at large,
>>>> but i think it may be important to not just introduce a yet another variant
>>>> of dealing with the same issue, but only have a single good modern way.
>>>
>>> Yes, I've seen DebugCounter. It's useful when you're already identified the file where something is going wrong, and you can enable the counters and pass counter values to opt. It doesn't however work across multiple TUs or support parallel debugging, so it's not solving the same problem.
>>
>> Perhaps it'd be worth considering the overlap here, and maybe avoiding it: What would it be like if this new thing /only/ bisected at the granularity of a whole compilation action (ie: one invocation of clang), rather than specific optimizations? (possibly even only at the "whole compiler" granularity - using two compilers - and choosing between one or the other for each compilation)
>>
>> Then once the specific file has been identified, use the existing sub-file granularity tools (a wrapper script could help cover both of these for the user so it wasn't a bunch more manual work).
>>
>> I think that might help prevent overlap of functionality/re-implementing similar/the same functionality through different mechanisms for reducing specific optimization applications?
>
> I think that restricting it to only allow bisection across translation units would be artificially constraining the functionality to not step on the toes of other features. That in itself doesn't seem the right approach, because the simplest thing right now is to support arbitrary bisection granularities. If we constrained it to only allow bisection down to TUs, then we haven't really simplified or re-used any code, all we've done is to make the user experience worse.
>
> I'll re-iterate that the purpose of using a daemon and not the existing file-level granularity tools like DebugCounter is to support parallelism and minimal intrusion into the build system. That's the guiding principle behind this patch. If other tools use file-granularity bisection using simple counters, then they should stick with those features, they're solving different problems.

Part of the direction is that it could influence/change the design of the feature, possibly to simplify it - for instance this could be implemented as a compiler wrapper (it could then even be compiler agnostic (maybe even tool agnostic/could be reused for other tooling investigations)) - for instance doing an A/B compiler comparison - the wrapper could do the network thing to check the key (keys would only be either input or output files (probably output files - input files could be ambiguous - the same input file might be rebuilt into multiple output targets with different arguments) and then run either one compiler or the other compiler - or otherwise massage the compiler arguments (disable an optimization, etc).

>> (also: probably an add-on for the future: Have you considered not requiring a clean build for each bisection step? Would it be feasible to have the bisect tool delete (or produce insturctions the user can copy/paste, if preferred) specific output files it knows it's going to change - then letting the user rerun an incremental build that will cause those files to be regenerated and allowing the bisect tool to intercept their building to adjust how they're built - it seems like this only requires a reliable build system (so it could be optional, so it could be turned off in cases where a build system doesn't reliably check for existing outputs) and could significantly improve performance of such a tool?)
>
> This is a really nice idea that I hadn't considered. If we can improve the API so that bisection daemon knows the semantics of the keys, then it should in theory be simple to have the daemon **touch** those translation unit sources to trigger the build system to rebuild. Deleting the object files seems a bit harder, since we'd have to propagate the argument to `-o` through to the bisection point, whereas the TU input is available from the IR module.

i'd worry the input could be ambiguous (same file built with different -D flags - admittedly not /really/ common) but the output is /probably/ less ambiguous (build system's unlikely to rebuild/replace a given output)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113030