[PATCH] D154084: [AArch64] Modify SVE Pseudo appends

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 29 07:29:24 PDT 2023


dmgreen added a reviewer: paulwalker-arm.
dmgreen added a comment.

Sounds like a good idea. And I like the test, that looks very useful in keeping the data insync.



================
Comment at: llvm/unittests/Target/AArch64/AArch64SvePseudoTest.cpp:34
+                      std::string(TM->getTargetFeatureString()), *TM,
+                      /* isLittle */ false);
+  return std::make_unique<AArch64InstrInfo>(ST);
----------------
false -> true


================
Comment at: llvm/unittests/Target/AArch64/AArch64SvePseudoTest.cpp:38
+
+TEST(AArch64SVEPseudoTest, IsCorrect) {
+  std::unique_ptr<LLVMTargetMachine> TM = createTargetMachine();
----------------
Can you add CortexA510 to this test name. It would be good to make use of the test in other cpu's like N2 and V2 (in a future patch). Perhaps pass the CPU as a parameter to createTargetMachine too?

And maybe make it clear that it is testing scheduling info. AArch64SVESchedPseudoTest or something similar?


================
Comment at: llvm/unittests/Target/AArch64/AArch64SvePseudoTest.cpp:51
+    // original instruction
+    int origInstr = AArch64::getSVEPseudoMap(i);
+    if (origInstr == -1)
----------------
origInstr -> OrigInstr, as per the llvm coding standard in https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly.


================
Comment at: llvm/unittests/Target/AArch64/AArch64SvePseudoTest.cpp:59
+
+    const MCInstrDesc &Descorig = II->get(origInstr);
+    unsigned SCClassorig = Descorig.getSchedClass();
----------------
Descorig -> DescOrig, same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154084



More information about the llvm-commits mailing list