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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/test/tools/llvm-reduce/mir/instr-reduce.mir:1
+# RUN: llvm-reduce -mir -mtriple=riscv32 --test %python --test-arg %p/instr-reduce.py %s -o %t
+# RUN: cat %t | FileCheck --match-full-lines %s
----------------
is it possible to do this via FileCheck like the other tests?


================
Comment at: llvm/tools/llvm-reduce/MyMachineModule.cpp:22
+
+static std::unique_ptr<MachineFunction> cloneMF(MachineFunction *SrcMF) {
+  auto DstMF = std::make_unique<MachineFunction>(
----------------
moving this inside `llvm/lib` would be nice


================
Comment at: llvm/tools/llvm-reduce/deltas/Delta.h:106
+    TestRunner &Test, int Targets,
+    std::function<void(const std::vector<Chunk> &, MyMachineModule *)>
+        ExtractChunksFromModule);
----------------
I'd prefer if this were still a `Module` param for normal passes to avoid the `MyMachineModule` abstraction inside the delta passes.
Is there a way to overload this with a `Module` and a `MachineFunction` version? e.g. forwarding them to an internal version of `runDeltaPass()` with wrappers to convert from a `MyMachineModule` to `Module`/`MachineFunction`


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.h:1
+//===- ReduceArguments.h - Specialized Delta Pass -------------------------===//
+//
----------------
needs updating


================
Comment at: llvm/tools/llvm-reduce/llvm-reduce.cpp:77
+static cl::opt<std::string>
+    TargetTriple("mtriple", cl::init(sys::getDefaultTargetTriple()),
+                 cl::desc("Set the target triple"));
----------------
will this work before initialization?


================
Comment at: llvm/tools/llvm-reduce/llvm-reduce.cpp:79
+                 cl::desc("Set the target triple"));
+cl::opt<bool> EnableMIR("mir", cl::desc("MIR mode"));
 
----------------
it'd be nice if we could automatically infer this from the input (like `llc`)


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

https://reviews.llvm.org/D110527