[PATCH] D110527: [llvm-reduce] Add MIR support.

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

In D110527#3080186 <https://reviews.llvm.org/D110527#3080186>, @markus wrote:

> In D110527#3079133 <https://reviews.llvm.org/D110527#3079133>, @lebedev.ri wrote:
>
>> I'm not sure i fully like the coupling here.
>> In reality, how much code duplication is being avoided by bundling these two tools together?
>
> Well, it is true that the llvm-reduce tool did not contain that many lines of code in the first place, and especially not if one excludes the IR specific reduction passes.
> There is the Delta Debugging Algorithm in Delta.cpp and some general stuff for handling input and output files as well as running the interestingness tests.
> In general code duplication is considered a bad thing so I think we all can agree that code reuse in some sense is desirable.
>
>> Would it not be better to extract the common interface and simplify writing new back/front -ends for llvm-reduce,
>> instead of having everything in a single tool?
>
> Could be but it also seems like that would be a much larger change (at least to the existing llvm-reduce). I am not particularly interested in doing that in this patch but as always anyone is free to evolve things further in a follow up commit if they consider it worth while.

I think currently the shared mir vs ir code doesn't hinder development too much. If it ever does come to that point we should split off an llvm-mir-reduce, but that can be done in the future.

(and sorry, I've been busy, trying to find time to review)



================
Comment at: llvm/tools/llvm-reduce/DeltaManager.cpp:42
                          "default, run all delta passes."));
+extern bool ReduceModeMIR;
 
----------------
this really shouldn't be a global, it should be passed in as a parameter


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp:84
+        ToDelete.insert(&MI);
+        // errs() << "Deleting: " << MI;
+      }
----------------
delete


================
Comment at: llvm/tools/llvm-reduce/llvm-reduce.cpp:140
+        InputLanguage != InputLanguages::MIR) {
+      errs() << Argv[0] << ": input language must be '', 'ir' or 'mir'\n";
+      return 1;
----------------
we shouldn't need this, I think the cl::opt will handle misspellings


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

https://reviews.llvm.org/D110527