Skip to content

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