[PATCH] D30521: Introduce llc/ExecuteTestCommands pass

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 2 09:14:24 PST 2017


MatzeB added inline comments.


================
Comment at: test/CodeGen/AMDGPU/handlemove.mir:1-12
+# RUN: llc -o - %s -mtriple=amdgcn -verify-machineinstrs -run-pass=execute-test-commands | FileCheck %s
+...
+name: MoveUpDef
+body: |
+  bb.0:
+    S_NOP 0
+    S_NOP 0
----------------
kristof.beyls wrote:
> Without reading the source of the rest of this patch, I find it hard to guess exactly what this is testing.
> Maybe before committing, also some documentation should be written on how to use this new feature?
> I'm not sure where the best place would be for that documentation to live. Maybe http://llvm.org/docs/MIRLangRef.html?
This means: "In basic block 0, take the 2nd instruction and move it before the first instruction". This will use the LiveIntervalAnalysis::handleMove() function which updates live intervals after moving an instruction around in a basic block, which was notoriously buggy and at the same time it was hard to created directed tests for it.

Yes, this is far from a clear and obvious solution. In an ideal solution we would have a way to name instructions instead of refering to them by the numbers 2 and 1. So there is room for further improvements, but that is for another patch.

This patch is about replacing the unittest code written in C++ which today looks like this:
```
TEST(LiveIntervalTest, MoveUpDef) {
  // Value defined.
  liveIntervalTest(R"MIR(
    S_NOP 0
    S_NOP 0
    early-clobber %0 = IMPLICIT_DEF
    S_NOP 0, implicit %0
)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
    testHandleMove(MF, LIS, 2, 1);
  });
}
```
which is not clearer but instead has more C++ boilerplate noise (and no FileCheck available).

I will add more documentation in the next iterations of this patch.



Repository:
  rL LLVM

https://reviews.llvm.org/D30521





More information about the llvm-commits mailing list