[PATCH] D68649: [Mips][llvm-exegesis] Add a Mips target
Clement Courbet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 9 00:57:01 PDT 2019
courbet added a comment.
Thanks for the contribution. I only have cosmetic comments.
================
Comment at: unittests/tools/llvm-exegesis/Mips/TargetTest.cpp:27
+
+using llvm::APInt;
+using llvm::MCInst;
----------------
You don't need these. I guess this is a copy-paste from the X86 tests: these are remains from a time when we were not in namespace llvm. (Fixed in rL374143).
================
Comment at: unittests/tools/llvm-exegesis/Mips/TargetTest.cpp:56
+TEST_F(MipsTargetTest, SetRegToConstant) {
+ const auto Insts = setRegTo(llvm::Mips::T0, llvm::APInt());
+ EXPECT_THAT(Insts, Not(IsEmpty()));
----------------
ditto: remove `llvm::`
================
Comment at: unittests/tools/llvm-exegesis/Mips/TargetTest.cpp:57
+ const auto Insts = setRegTo(llvm::Mips::T0, llvm::APInt());
+ EXPECT_THAT(Insts, Not(IsEmpty()));
+}
----------------
Don't you want to test the actual instruction ?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68649/new/
https://reviews.llvm.org/D68649
More information about the llvm-commits
mailing list