[PATCH] D96673: [GlobalISel] Implement narrowScalar for SADDE/SSUBE/UADDE/USUBE

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 18 18:36:52 PST 2021


arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

LGTM with test nits



================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:1052-1053
+  LegalizerHelper Helper(*MF, Info, Observer, B);
+  EXPECT_TRUE(Helper.narrowScalar(*UADDE, 0, S32) ==
+              LegalizerHelper::LegalizeResult::Legalized);
+
----------------
EXPECT_EQ(Legalized, ...)


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:1078-1081
+  DefineLegalizerInfo(A, {
+    getActionDefinitionsBuilder(G_USUBE).legalFor(
+        {{LLT::scalar(32), LLT::scalar(1)}});
+  });
----------------
You don't actually need to define the legalizer rule since you are directly calling the implementation


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:1091-1092
+  LegalizerHelper Helper(*MF, Info, Observer, B);
+  EXPECT_TRUE(Helper.narrowScalar(*USUBE, 0, S32) ==
+              LegalizerHelper::LegalizeResult::Legalized);
+
----------------
EXPECT_EQ


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:1130-1131
+  LegalizerHelper Helper(*MF, Info, Observer, B);
+  EXPECT_TRUE(Helper.narrowScalar(*SADDE, 0, S32) ==
+              LegalizerHelper::LegalizeResult::Legalized);
+
----------------
EXPECT_EQ


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:1169-1170
+  LegalizerHelper Helper(*MF, Info, Observer, B);
+  EXPECT_TRUE(Helper.narrowScalar(*SSUBE, 0, S32) ==
+              LegalizerHelper::LegalizeResult::Legalized);
+
----------------
EXPECT_EQ


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96673



More information about the llvm-commits mailing list