[PATCH] D48020: [llvm-exegesis] Cleaner design without mutable data.

Clement Courbet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 13 05:10:21 PDT 2018


courbet added inline comments.


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:66
+  EXPECT_THAT(Conf.Info, testing::HasSubstr("implicit"));
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(1));
+  const llvm::MCInst Instr = Conf.Snippet[0];
----------------
ASSERT_THAT, since you're taking the front below.


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:87
+  EXPECT_THAT(Conf.Info, testing::HasSubstr("explicit"));
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(1));
+  const llvm::MCInst Instr = Conf.Snippet[0];
----------------
ditto


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:115
+  EXPECT_THAT(Conf.Info, testing::HasSubstr("cycle through CMOVLE16rr"));
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(2));
+
----------------
ditto


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:125
+  const llvm::MCInst InstrB = Conf.Snippet[1];
+  EXPECT_THAT(InstrB.getOpcode(), llvm::X86::CMOVLE16rr);
+  EXPECT_THAT(InstrB.getNumOperands(), 3);
----------------
should you be testing that ? this is randomized.


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:157
+  EXPECT_THAT(Conf.Info, testing::HasSubstr("parallel"));
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(1));
+  const llvm::MCInst Instr = Conf.Snippet[0];
----------------
ditto


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:174
+  EXPECT_THAT(Conf.Info, testing::HasSubstr("serial"));
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(1));
+  const llvm::MCInst Instr = Conf.Snippet[0];
----------------
ditto


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:192
+  constexpr const unsigned kInstructionCount = 15;
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(kInstructionCount));
+  struct Operands {
----------------
ditto


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:193
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(kInstructionCount));
+  struct Operands {
+    unsigned Op01;
----------------
Same here: any ordering would be OK.


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:228
+  EXPECT_THAT(Conf.Info, testing::HasSubstr("no tied variables"));
+  EXPECT_THAT(Conf.Snippet, testing::SizeIs(1));
+  const llvm::MCInst Instr = Conf.Snippet[0];
----------------
ditto


================
Comment at: unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp:232
+  EXPECT_THAT(Instr.getNumOperands(), 4);
+  EXPECT_THAT(Instr.getOperand(0).getReg(), llvm::X86::R9D);
+  EXPECT_THAT(Instr.getOperand(1).getReg(), llvm::X86::R10D);
----------------
i don;t think you shoul dbe testing the exact assignment


Repository:
  rL LLVM

https://reviews.llvm.org/D48020





More information about the llvm-commits mailing list