Skip to content

[RISCV][GISel] Use boolean predicated legalization action methods to simplify code. NFC #115063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Nov 5, 2024

These allow us to pass a subtarget feature to conditionally enable the legalization action.

These were added by a3010c7 and are used by AArch64.

…simplify code. NFC

These allow us to pass a subtarget feature to conditionally enable
the legalization action.

These were added by a3010c7 and
are used by AArch64.
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

These allow us to pass a subtarget feature to conditionally enable the legalization action.

These were added by a3010c7 and are used by AArch64.


Full diff: https://github.com/llvm/llvm-project/pull/115063.diff

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+25-28)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index 50d95d7695e0ff..f2a51a7ea0d426 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -157,8 +157,8 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
   getActionDefinitionsBuilder({G_UADDSAT, G_SADDSAT, G_USUBSAT, G_SSUBSAT})
       .lower();
 
-  auto &ShiftActions = getActionDefinitionsBuilder({G_ASHR, G_LSHR, G_SHL});
-  ShiftActions.legalFor({{s32, s32}, {sXLen, sXLen}})
+  getActionDefinitionsBuilder({G_ASHR, G_LSHR, G_SHL})
+      .legalFor({{s32, s32}, {sXLen, sXLen}})
       .widenScalarToNextPow2(0)
       .clampScalar(1, s32, sXLen)
       .clampScalar(0, s32, sXLen)
@@ -201,10 +201,10 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
 
   getActionDefinitionsBuilder({G_FSHL, G_FSHR}).lower();
 
-  auto &RotateActions = getActionDefinitionsBuilder({G_ROTL, G_ROTR});
-  if (ST.hasStdExtZbb() || ST.hasStdExtZbkb())
-    RotateActions.legalFor({{s32, s32}, {sXLen, sXLen}});
-  RotateActions.lower();
+  getActionDefinitionsBuilder({G_ROTL, G_ROTR})
+      .legalFor(ST.hasStdExtZbb() || ST.hasStdExtZbkb(),
+                {{s32, s32}, {sXLen, sXLen}})
+      .lower();
 
   getActionDefinitionsBuilder(G_BITREVERSE).maxScalar(0, sXLen).lower();
 
@@ -244,11 +244,11 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
     CTPOPActions.maxScalar(0, sXLen).scalarSameSizeAs(1, 0).lower();
   }
 
-  auto &ConstantActions = getActionDefinitionsBuilder(G_CONSTANT);
-  ConstantActions.legalFor({s32, p0});
-  if (ST.is64Bit())
-    ConstantActions.customFor({s64});
-  ConstantActions.widenScalarToNextPow2(0).clampScalar(0, s32, sXLen);
+  getActionDefinitionsBuilder(G_CONSTANT)
+      .legalFor({s32, p0})
+      .customFor(ST.is64Bit(), {s64})
+      .widenScalarToNextPow2(0)
+      .clampScalar(0, s32, sXLen);
 
   // TODO: transform illegal vector types into legal vector type
   getActionDefinitionsBuilder(
@@ -267,14 +267,12 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
       .clampScalar(1, sXLen, sXLen)
       .clampScalar(0, sXLen, sXLen);
 
-  auto &SelectActions =
-      getActionDefinitionsBuilder(G_SELECT)
-          .legalFor({{s32, sXLen}, {p0, sXLen}})
-          .legalIf(all(typeIsLegalIntOrFPVec(0, IntOrFPVecTys, ST),
-                       typeIsLegalBoolVec(1, BoolVecTys, ST)));
-  if (XLen == 64 || ST.hasStdExtD())
-    SelectActions.legalFor({{s64, sXLen}});
-  SelectActions.widenScalarToNextPow2(0)
+  getActionDefinitionsBuilder(G_SELECT)
+      .legalFor({{s32, sXLen}, {p0, sXLen}})
+      .legalIf(all(typeIsLegalIntOrFPVec(0, IntOrFPVecTys, ST),
+                   typeIsLegalBoolVec(1, BoolVecTys, ST)))
+      .legalFor(XLen == 64 || ST.hasStdExtD(), {{s64, sXLen}})
+      .widenScalarToNextPow2(0)
       .clampScalar(0, s32, (XLen == 64 || ST.hasStdExtD()) ? s64 : s32)
       .clampScalar(1, sXLen, sXLen);
 
@@ -471,16 +469,15 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
   // TODO: Use libcall for sDoubleXLen.
   getActionDefinitionsBuilder({G_UDIVREM, G_SDIVREM}).lower();
 
-  auto &AbsActions = getActionDefinitionsBuilder(G_ABS);
-  if (ST.hasStdExtZbb())
-    AbsActions.customFor({sXLen}).minScalar(0, sXLen);
-  AbsActions.lower();
+  getActionDefinitionsBuilder(G_ABS)
+      .customFor(ST.hasStdExtZbb(), {sXLen})
+      .minScalar(ST.hasStdExtZbb(), 0, sXLen)
+      .lower();
 
-  auto &MinMaxActions =
-      getActionDefinitionsBuilder({G_UMAX, G_UMIN, G_SMAX, G_SMIN});
-  if (ST.hasStdExtZbb())
-    MinMaxActions.legalFor({sXLen}).minScalar(0, sXLen);
-  MinMaxActions.lower();
+  getActionDefinitionsBuilder({G_UMAX, G_UMIN, G_SMAX, G_SMIN})
+      .legalFor(ST.hasStdExtZbb(), {sXLen})
+      .minScalar(ST.hasStdExtZbb(), 0, sXLen)
+      .lower();
 
   getActionDefinitionsBuilder(G_FRAME_INDEX).legalFor({p0});
 

@lenary
Copy link
Member

lenary commented Nov 5, 2024

Is there any value in our backend having a test like llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir? If we think so, we're probably going to need one for each feature combination used in these predicates.

@topperc topperc merged commit 3163f83 into llvm:main Nov 5, 2024
8 of 10 checks passed
@topperc topperc deleted the pr/gisel-bool-predicates branch November 5, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants