[GlobalISel][RISCV] Use constant pool for large integer constants. #81101

mgudim · 2024-02-08T07:42:15Z

We apply custom lowering to 64 bit constants where we use the same logic as in non-global isel: if materializing in registers is too expensive, we emit a load from constant pool. Later, during instruction selection, constant pool address is generated using selectAddr.

We apply custom lowering to 64 bit constants where we use the same logic as in non-global isel: if materializing in registers is too expensive, we emit a load from constant pool. Later, during instruction selection, constant pool address is generated using `selectAddr`.

llvmbot · 2024-02-08T07:42:47Z

@llvm/pr-subscribers-llvm-globalisel

Author: Mikhail Gudim (mgudim)

Changes

We apply custom lowering to 64 bit constants where we use the same logic as in non-global isel: if materializing in registers is too expensive, we emit a load from constant pool. Later, during instruction selection, constant pool address is generated using selectAddr.

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

4 Files Affected:

(modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+90-1)
(modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h (+4)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir (+18-15)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir (+12-8)

diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index ae02e86baf6e8b..e238a028706fc3 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -11,11 +11,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "RISCVLegalizerInfo.h"
+#include "MCTargetDesc/RISCVMatInt.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVSubtarget.h"
+#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
 #include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetOpcodes.h"
 #include "llvm/CodeGen/ValueTypes.h"
@@ -182,7 +185,13 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
     CTPOPActions.maxScalar(0, sXLen).scalarSameSizeAs(1, 0).lower();
   }
 
-  getActionDefinitionsBuilder({G_CONSTANT, G_IMPLICIT_DEF})
+  auto &ConstantActions = getActionDefinitionsBuilder(G_CONSTANT);
+  if (ST.is64Bit())
+    ConstantActions.customFor({s64});
+  ConstantActions.legalFor({s32, p0}).widenScalarToNextPow2(0).clampScalar(
+      0, s32, sXLen);
+
+  getActionDefinitionsBuilder(G_IMPLICIT_DEF)
       .legalFor({s32, sXLen, p0})
       .widenScalarToNextPow2(0)
       .clampScalar(0, s32, sXLen);
@@ -451,17 +460,97 @@ bool RISCVLegalizerInfo::legalizeVAStart(MachineInstr &MI,
   return true;
 }
 
+bool RISCVLegalizerInfo::shouldBeInConstantPool(APInt APImm,
+                                                bool shouldOptForSize) const {
+  unsigned BitWidth = APImm.getBitWidth();
+  assert(BitWidth == 32 || BitWidth == 64);
+  uint64_t Imm = APImm.getZExtValue();
+  // All simm32 constants should be handled by isel.
+  // NOTE: The getMaxBuildIntsCost call below should return a value >= 2 making
+  // this check redundant, but small immediates are common so this check
+  // should have better compile time.
+  if (isInt<32>(Imm))
+    return false;
+
+  // We only need to cost the immediate, if constant pool lowering is enabled.
+  if (!STI.useConstantPoolForLargeInts())
+    return false;
+
+  RISCVMatInt::InstSeq Seq = RISCVMatInt::generateInstSeq(Imm, STI);
+  if (Seq.size() <= STI.getMaxBuildIntsCost())
+    return false;
+
+  // Optimizations below are disabled for opt size. If we're optimizing for
+  // size, use a constant pool.
+  if (shouldOptForSize)
+    return true;
+  //
+  // Special case. See if we can build the constant as (ADD (SLLI X, C), X) do
+  // that if it will avoid a constant pool.
+  // It will require an extra temporary register though.
+  // If we have Zba we can use (ADD_UW X, (SLLI X, 32)) to handle cases where
+  // low and high 32 bits are the same and bit 31 and 63 are set.
+  unsigned ShiftAmt, AddOpc;
+  RISCVMatInt::InstSeq SeqLo =
+      RISCVMatInt::generateTwoRegInstSeq(Imm, STI, ShiftAmt, AddOpc);
+  if (!SeqLo.empty() && (SeqLo.size() + 2) <= STI.getMaxBuildIntsCost())
+    return false;
+
+  return true;
+}
+
+// TODO: This is almost the same as LegalizerHelper::lowerFConstant and is
+// target-independent. Should we move this to LegalizeHelper?
+bool RISCVLegalizerInfo::emitLoadFromConstantPool(
+    Register DstReg, const Constant *ConstVal,
+    MachineIRBuilder &MIRBuilder) const {
+  MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
+  MachineFunction &MF = MIRBuilder.getMF();
+  const DataLayout &DL = MIRBuilder.getDataLayout();
+  LLVMContext &Ctx = MF.getFunction().getContext();
+  unsigned AddrSpace = DL.getDefaultGlobalsAddressSpace();
+  LLT AddrPtrTy = LLT::pointer(AddrSpace, DL.getPointerSizeInBits(AddrSpace));
+  LLT DstLLT = MRI.getType(DstReg);
+
+  Align Alignment = Align(DL.getABITypeAlign(getTypeForLLT(DstLLT, Ctx)));
+
+  auto Addr = MIRBuilder.buildConstantPool(
+      AddrPtrTy,
+      MF.getConstantPool()->getConstantPoolIndex(ConstVal, Alignment));
+  MachineMemOperand *MMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getConstantPool(MF),
+                              MachineMemOperand::MOLoad, DstLLT, Alignment);
+
+  MIRBuilder.buildLoadInstr(TargetOpcode::G_LOAD, DstReg, Addr, *MMO);
+  return true;
+}
+
 bool RISCVLegalizerInfo::legalizeCustom(
     LegalizerHelper &Helper, MachineInstr &MI,
     LostDebugLocObserver &LocObserver) const {
   MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
   GISelChangeObserver &Observer = Helper.Observer;
+  MachineFunction &MF = *MI.getParent()->getParent();
   switch (MI.getOpcode()) {
   default:
     // No idea what to do.
     return false;
   case TargetOpcode::G_ABS:
     return Helper.lowerAbsToMaxNeg(MI);
+  // TODO: G_FCONSTANT
+  case TargetOpcode::G_CONSTANT: {
+    const Function &F = MF.getFunction();
+    // TODO: if PSI and BFI are present, add " ||
+    // llvm::shouldOptForSize(*CurMBB, PSI, BFI)".
+    bool shouldOptForSize = F.hasOptSize() || F.hasMinSize();
+    const ConstantInt *ConstVal = MI.getOperand(1).getCImm();
+    if (!shouldBeInConstantPool(ConstVal->getValue(), shouldOptForSize))
+      return true;
+    emitLoadFromConstantPool(MI.getOperand(0).getReg(),
+                             MI.getOperand(1).getCImm(), MIRBuilder);
+    MI.eraseFromParent();
+    return true;
+  }
   case TargetOpcode::G_SHL:
   case TargetOpcode::G_ASHR:
   case TargetOpcode::G_LSHR:
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
index f3ec6be167347e..2845902d3d3ab3 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
@@ -14,6 +14,7 @@
 #define LLVM_LIB_TARGET_RISCV_RISCVMACHINELEGALIZER_H
 
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/Register.h"
 
 namespace llvm {
 
@@ -36,6 +37,9 @@ class RISCVLegalizerInfo : public LegalizerInfo {
                          MachineInstr &MI) const override;
 
 private:
+  bool shouldBeInConstantPool(APInt APImm, bool shouldOptForSize) const;
+  bool emitLoadFromConstantPool(Register DstReg, const Constant *CPVal,
+                                MachineIRBuilder &MIRBuilder) const;
   bool legalizeShlAshrLshr(MachineInstr &MI, MachineIRBuilder &MIRBuilder,
                            GISelChangeObserver &Observer) const;
 
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
index f4a098dd28c418..d1473504651668 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
@@ -220,25 +220,28 @@ body:             |
     ; CHECK-NEXT: [[AND5:%[0-9]+]]:_(s64) = G_AND [[LSHR3]], [[C5]]
     ; CHECK-NEXT: [[OR6:%[0-9]+]]:_(s64) = G_OR [[OR5]], [[AND5]]
     ; CHECK-NEXT: [[C7:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
-    ; CHECK-NEXT: [[C8:%[0-9]+]]:_(s64) = G_CONSTANT i64 -1085102592571150096
-    ; CHECK-NEXT: [[AND6:%[0-9]+]]:_(s64) = G_AND [[OR6]], [[C8]]
+    ; CHECK-NEXT: [[CONSTANT_POOL:%[0-9]+]]:_(p0) = G_CONSTANT_POOL %const.2
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s64) = G_LOAD [[CONSTANT_POOL]](p0) :: (load (s64) from constant-pool)
+    ; CHECK-NEXT: [[AND6:%[0-9]+]]:_(s64) = G_AND [[OR6]], [[LOAD]]
     ; CHECK-NEXT: [[LSHR4:%[0-9]+]]:_(s64) = G_LSHR [[AND6]], [[C7]](s64)
     ; CHECK-NEXT: [[SHL4:%[0-9]+]]:_(s64) = G_SHL [[OR6]], [[C7]](s64)
-    ; CHECK-NEXT: [[AND7:%[0-9]+]]:_(s64) = G_AND [[SHL4]], [[C8]]
+    ; CHECK-NEXT: [[AND7:%[0-9]+]]:_(s64) = G_AND [[SHL4]], [[LOAD]]
     ; CHECK-NEXT: [[OR7:%[0-9]+]]:_(s64) = G_OR [[LSHR4]], [[AND7]]
-    ; CHECK-NEXT: [[C9:%[0-9]+]]:_(s64) = G_CONSTANT i64 2
-    ; CHECK-NEXT: [[C10:%[0-9]+]]:_(s64) = G_CONSTANT i64 -3689348814741910324
-    ; CHECK-NEXT: [[AND8:%[0-9]+]]:_(s64) = G_AND [[OR7]], [[C10]]
-    ; CHECK-NEXT: [[LSHR5:%[0-9]+]]:_(s64) = G_LSHR [[AND8]], [[C9]](s64)
-    ; CHECK-NEXT: [[SHL5:%[0-9]+]]:_(s64) = G_SHL [[OR7]], [[C9]](s64)
-    ; CHECK-NEXT: [[AND9:%[0-9]+]]:_(s64) = G_AND [[SHL5]], [[C10]]
+    ; CHECK-NEXT: [[C8:%[0-9]+]]:_(s64) = G_CONSTANT i64 2
+    ; CHECK-NEXT: [[CONSTANT_POOL1:%[0-9]+]]:_(p0) = G_CONSTANT_POOL %const.1
+    ; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(s64) = G_LOAD [[CONSTANT_POOL1]](p0) :: (load (s64) from constant-pool)
+    ; CHECK-NEXT: [[AND8:%[0-9]+]]:_(s64) = G_AND [[OR7]], [[LOAD1]]
+    ; CHECK-NEXT: [[LSHR5:%[0-9]+]]:_(s64) = G_LSHR [[AND8]], [[C8]](s64)
+    ; CHECK-NEXT: [[SHL5:%[0-9]+]]:_(s64) = G_SHL [[OR7]], [[C8]](s64)
+    ; CHECK-NEXT: [[AND9:%[0-9]+]]:_(s64) = G_AND [[SHL5]], [[LOAD1]]
     ; CHECK-NEXT: [[OR8:%[0-9]+]]:_(s64) = G_OR [[LSHR5]], [[AND9]]
-    ; CHECK-NEXT: [[C11:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
-    ; CHECK-NEXT: [[C12:%[0-9]+]]:_(s64) = G_CONSTANT i64 -6148914691236517206
-    ; CHECK-NEXT: [[AND10:%[0-9]+]]:_(s64) = G_AND [[OR8]], [[C12]]
-    ; CHECK-NEXT: [[LSHR6:%[0-9]+]]:_(s64) = G_LSHR [[AND10]], [[C11]](s64)
-    ; CHECK-NEXT: [[SHL6:%[0-9]+]]:_(s64) = G_SHL [[OR8]], [[C11]](s64)
-    ; CHECK-NEXT: [[AND11:%[0-9]+]]:_(s64) = G_AND [[SHL6]], [[C12]]
+    ; CHECK-NEXT: [[C9:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
+    ; CHECK-NEXT: [[CONSTANT_POOL2:%[0-9]+]]:_(p0) = G_CONSTANT_POOL %const.0
+    ; CHECK-NEXT: [[LOAD2:%[0-9]+]]:_(s64) = G_LOAD [[CONSTANT_POOL2]](p0) :: (load (s64) from constant-pool)
+    ; CHECK-NEXT: [[AND10:%[0-9]+]]:_(s64) = G_AND [[OR8]], [[LOAD2]]
+    ; CHECK-NEXT: [[LSHR6:%[0-9]+]]:_(s64) = G_LSHR [[AND10]], [[C9]](s64)
+    ; CHECK-NEXT: [[SHL6:%[0-9]+]]:_(s64) = G_SHL [[OR8]], [[C9]](s64)
+    ; CHECK-NEXT: [[AND11:%[0-9]+]]:_(s64) = G_AND [[SHL6]], [[LOAD2]]
     ; CHECK-NEXT: [[OR9:%[0-9]+]]:_(s64) = G_OR [[LSHR6]], [[AND11]]
     ; CHECK-NEXT: $x10 = COPY [[OR9]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir
index fa57295d3a540f..63f93d13c04633 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir
@@ -6,8 +6,9 @@ name:            const_i8
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i8
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 -127
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 -127
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s8) = G_CONSTANT i8 129
     %1:_(s64) = G_ANYEXT %0(s8)
@@ -20,8 +21,9 @@ name:            const_i15
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i15
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 15
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 15
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s15) = G_CONSTANT i15 15
     %1:_(s64) = G_ANYEXT %0(s15)
@@ -34,8 +36,9 @@ name:            const_i16
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i16
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 767
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 767
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s16) = G_CONSTANT i16 -64769
     %1:_(s64) = G_ANYEXT %0(s16)
@@ -48,8 +51,9 @@ name:            const_i32
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i32
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 -64769
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 -64769
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s32) = G_CONSTANT i32 -64769
     %1:_(s64) = G_ANYEXT %0(s32)

llvmbot · 2024-02-08T07:42:47Z

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

Author: Mikhail Gudim (mgudim)

Changes

We apply custom lowering to 64 bit constants where we use the same logic as in non-global isel: if materializing in registers is too expensive, we emit a load from constant pool. Later, during instruction selection, constant pool address is generated using selectAddr.

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

4 Files Affected:

(modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+90-1)
(modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h (+4)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir (+18-15)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir (+12-8)

diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index ae02e86baf6e8b..e238a028706fc3 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -11,11 +11,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "RISCVLegalizerInfo.h"
+#include "MCTargetDesc/RISCVMatInt.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVSubtarget.h"
+#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
 #include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetOpcodes.h"
 #include "llvm/CodeGen/ValueTypes.h"
@@ -182,7 +185,13 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
     CTPOPActions.maxScalar(0, sXLen).scalarSameSizeAs(1, 0).lower();
   }
 
-  getActionDefinitionsBuilder({G_CONSTANT, G_IMPLICIT_DEF})
+  auto &ConstantActions = getActionDefinitionsBuilder(G_CONSTANT);
+  if (ST.is64Bit())
+    ConstantActions.customFor({s64});
+  ConstantActions.legalFor({s32, p0}).widenScalarToNextPow2(0).clampScalar(
+      0, s32, sXLen);
+
+  getActionDefinitionsBuilder(G_IMPLICIT_DEF)
       .legalFor({s32, sXLen, p0})
       .widenScalarToNextPow2(0)
       .clampScalar(0, s32, sXLen);
@@ -451,17 +460,97 @@ bool RISCVLegalizerInfo::legalizeVAStart(MachineInstr &MI,
   return true;
 }
 
+bool RISCVLegalizerInfo::shouldBeInConstantPool(APInt APImm,
+                                                bool shouldOptForSize) const {
+  unsigned BitWidth = APImm.getBitWidth();
+  assert(BitWidth == 32 || BitWidth == 64);
+  uint64_t Imm = APImm.getZExtValue();
+  // All simm32 constants should be handled by isel.
+  // NOTE: The getMaxBuildIntsCost call below should return a value >= 2 making
+  // this check redundant, but small immediates are common so this check
+  // should have better compile time.
+  if (isInt<32>(Imm))
+    return false;
+
+  // We only need to cost the immediate, if constant pool lowering is enabled.
+  if (!STI.useConstantPoolForLargeInts())
+    return false;
+
+  RISCVMatInt::InstSeq Seq = RISCVMatInt::generateInstSeq(Imm, STI);
+  if (Seq.size() <= STI.getMaxBuildIntsCost())
+    return false;
+
+  // Optimizations below are disabled for opt size. If we're optimizing for
+  // size, use a constant pool.
+  if (shouldOptForSize)
+    return true;
+  //
+  // Special case. See if we can build the constant as (ADD (SLLI X, C), X) do
+  // that if it will avoid a constant pool.
+  // It will require an extra temporary register though.
+  // If we have Zba we can use (ADD_UW X, (SLLI X, 32)) to handle cases where
+  // low and high 32 bits are the same and bit 31 and 63 are set.
+  unsigned ShiftAmt, AddOpc;
+  RISCVMatInt::InstSeq SeqLo =
+      RISCVMatInt::generateTwoRegInstSeq(Imm, STI, ShiftAmt, AddOpc);
+  if (!SeqLo.empty() && (SeqLo.size() + 2) <= STI.getMaxBuildIntsCost())
+    return false;
+
+  return true;
+}
+
+// TODO: This is almost the same as LegalizerHelper::lowerFConstant and is
+// target-independent. Should we move this to LegalizeHelper?
+bool RISCVLegalizerInfo::emitLoadFromConstantPool(
+    Register DstReg, const Constant *ConstVal,
+    MachineIRBuilder &MIRBuilder) const {
+  MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
+  MachineFunction &MF = MIRBuilder.getMF();
+  const DataLayout &DL = MIRBuilder.getDataLayout();
+  LLVMContext &Ctx = MF.getFunction().getContext();
+  unsigned AddrSpace = DL.getDefaultGlobalsAddressSpace();
+  LLT AddrPtrTy = LLT::pointer(AddrSpace, DL.getPointerSizeInBits(AddrSpace));
+  LLT DstLLT = MRI.getType(DstReg);
+
+  Align Alignment = Align(DL.getABITypeAlign(getTypeForLLT(DstLLT, Ctx)));
+
+  auto Addr = MIRBuilder.buildConstantPool(
+      AddrPtrTy,
+      MF.getConstantPool()->getConstantPoolIndex(ConstVal, Alignment));
+  MachineMemOperand *MMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getConstantPool(MF),
+                              MachineMemOperand::MOLoad, DstLLT, Alignment);
+
+  MIRBuilder.buildLoadInstr(TargetOpcode::G_LOAD, DstReg, Addr, *MMO);
+  return true;
+}
+
 bool RISCVLegalizerInfo::legalizeCustom(
     LegalizerHelper &Helper, MachineInstr &MI,
     LostDebugLocObserver &LocObserver) const {
   MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
   GISelChangeObserver &Observer = Helper.Observer;
+  MachineFunction &MF = *MI.getParent()->getParent();
   switch (MI.getOpcode()) {
   default:
     // No idea what to do.
     return false;
   case TargetOpcode::G_ABS:
     return Helper.lowerAbsToMaxNeg(MI);
+  // TODO: G_FCONSTANT
+  case TargetOpcode::G_CONSTANT: {
+    const Function &F = MF.getFunction();
+    // TODO: if PSI and BFI are present, add " ||
+    // llvm::shouldOptForSize(*CurMBB, PSI, BFI)".
+    bool shouldOptForSize = F.hasOptSize() || F.hasMinSize();
+    const ConstantInt *ConstVal = MI.getOperand(1).getCImm();
+    if (!shouldBeInConstantPool(ConstVal->getValue(), shouldOptForSize))
+      return true;
+    emitLoadFromConstantPool(MI.getOperand(0).getReg(),
+                             MI.getOperand(1).getCImm(), MIRBuilder);
+    MI.eraseFromParent();
+    return true;
+  }
   case TargetOpcode::G_SHL:
   case TargetOpcode::G_ASHR:
   case TargetOpcode::G_LSHR:
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
index f3ec6be167347e..2845902d3d3ab3 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
@@ -14,6 +14,7 @@
 #define LLVM_LIB_TARGET_RISCV_RISCVMACHINELEGALIZER_H
 
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/Register.h"
 
 namespace llvm {
 
@@ -36,6 +37,9 @@ class RISCVLegalizerInfo : public LegalizerInfo {
                          MachineInstr &MI) const override;
 
 private:
+  bool shouldBeInConstantPool(APInt APImm, bool shouldOptForSize) const;
+  bool emitLoadFromConstantPool(Register DstReg, const Constant *CPVal,
+                                MachineIRBuilder &MIRBuilder) const;
   bool legalizeShlAshrLshr(MachineInstr &MI, MachineIRBuilder &MIRBuilder,
                            GISelChangeObserver &Observer) const;
 
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
index f4a098dd28c418..d1473504651668 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
@@ -220,25 +220,28 @@ body:             |
     ; CHECK-NEXT: [[AND5:%[0-9]+]]:_(s64) = G_AND [[LSHR3]], [[C5]]
     ; CHECK-NEXT: [[OR6:%[0-9]+]]:_(s64) = G_OR [[OR5]], [[AND5]]
     ; CHECK-NEXT: [[C7:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
-    ; CHECK-NEXT: [[C8:%[0-9]+]]:_(s64) = G_CONSTANT i64 -1085102592571150096
-    ; CHECK-NEXT: [[AND6:%[0-9]+]]:_(s64) = G_AND [[OR6]], [[C8]]
+    ; CHECK-NEXT: [[CONSTANT_POOL:%[0-9]+]]:_(p0) = G_CONSTANT_POOL %const.2
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s64) = G_LOAD [[CONSTANT_POOL]](p0) :: (load (s64) from constant-pool)
+    ; CHECK-NEXT: [[AND6:%[0-9]+]]:_(s64) = G_AND [[OR6]], [[LOAD]]
     ; CHECK-NEXT: [[LSHR4:%[0-9]+]]:_(s64) = G_LSHR [[AND6]], [[C7]](s64)
     ; CHECK-NEXT: [[SHL4:%[0-9]+]]:_(s64) = G_SHL [[OR6]], [[C7]](s64)
-    ; CHECK-NEXT: [[AND7:%[0-9]+]]:_(s64) = G_AND [[SHL4]], [[C8]]
+    ; CHECK-NEXT: [[AND7:%[0-9]+]]:_(s64) = G_AND [[SHL4]], [[LOAD]]
     ; CHECK-NEXT: [[OR7:%[0-9]+]]:_(s64) = G_OR [[LSHR4]], [[AND7]]
-    ; CHECK-NEXT: [[C9:%[0-9]+]]:_(s64) = G_CONSTANT i64 2
-    ; CHECK-NEXT: [[C10:%[0-9]+]]:_(s64) = G_CONSTANT i64 -3689348814741910324
-    ; CHECK-NEXT: [[AND8:%[0-9]+]]:_(s64) = G_AND [[OR7]], [[C10]]
-    ; CHECK-NEXT: [[LSHR5:%[0-9]+]]:_(s64) = G_LSHR [[AND8]], [[C9]](s64)
-    ; CHECK-NEXT: [[SHL5:%[0-9]+]]:_(s64) = G_SHL [[OR7]], [[C9]](s64)
-    ; CHECK-NEXT: [[AND9:%[0-9]+]]:_(s64) = G_AND [[SHL5]], [[C10]]
+    ; CHECK-NEXT: [[C8:%[0-9]+]]:_(s64) = G_CONSTANT i64 2
+    ; CHECK-NEXT: [[CONSTANT_POOL1:%[0-9]+]]:_(p0) = G_CONSTANT_POOL %const.1
+    ; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(s64) = G_LOAD [[CONSTANT_POOL1]](p0) :: (load (s64) from constant-pool)
+    ; CHECK-NEXT: [[AND8:%[0-9]+]]:_(s64) = G_AND [[OR7]], [[LOAD1]]
+    ; CHECK-NEXT: [[LSHR5:%[0-9]+]]:_(s64) = G_LSHR [[AND8]], [[C8]](s64)
+    ; CHECK-NEXT: [[SHL5:%[0-9]+]]:_(s64) = G_SHL [[OR7]], [[C8]](s64)
+    ; CHECK-NEXT: [[AND9:%[0-9]+]]:_(s64) = G_AND [[SHL5]], [[LOAD1]]
     ; CHECK-NEXT: [[OR8:%[0-9]+]]:_(s64) = G_OR [[LSHR5]], [[AND9]]
-    ; CHECK-NEXT: [[C11:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
-    ; CHECK-NEXT: [[C12:%[0-9]+]]:_(s64) = G_CONSTANT i64 -6148914691236517206
-    ; CHECK-NEXT: [[AND10:%[0-9]+]]:_(s64) = G_AND [[OR8]], [[C12]]
-    ; CHECK-NEXT: [[LSHR6:%[0-9]+]]:_(s64) = G_LSHR [[AND10]], [[C11]](s64)
-    ; CHECK-NEXT: [[SHL6:%[0-9]+]]:_(s64) = G_SHL [[OR8]], [[C11]](s64)
-    ; CHECK-NEXT: [[AND11:%[0-9]+]]:_(s64) = G_AND [[SHL6]], [[C12]]
+    ; CHECK-NEXT: [[C9:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
+    ; CHECK-NEXT: [[CONSTANT_POOL2:%[0-9]+]]:_(p0) = G_CONSTANT_POOL %const.0
+    ; CHECK-NEXT: [[LOAD2:%[0-9]+]]:_(s64) = G_LOAD [[CONSTANT_POOL2]](p0) :: (load (s64) from constant-pool)
+    ; CHECK-NEXT: [[AND10:%[0-9]+]]:_(s64) = G_AND [[OR8]], [[LOAD2]]
+    ; CHECK-NEXT: [[LSHR6:%[0-9]+]]:_(s64) = G_LSHR [[AND10]], [[C9]](s64)
+    ; CHECK-NEXT: [[SHL6:%[0-9]+]]:_(s64) = G_SHL [[OR8]], [[C9]](s64)
+    ; CHECK-NEXT: [[AND11:%[0-9]+]]:_(s64) = G_AND [[SHL6]], [[LOAD2]]
     ; CHECK-NEXT: [[OR9:%[0-9]+]]:_(s64) = G_OR [[LSHR6]], [[AND11]]
     ; CHECK-NEXT: $x10 = COPY [[OR9]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir
index fa57295d3a540f..63f93d13c04633 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-const-rv64.mir
@@ -6,8 +6,9 @@ name:            const_i8
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i8
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 -127
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 -127
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s8) = G_CONSTANT i8 129
     %1:_(s64) = G_ANYEXT %0(s8)
@@ -20,8 +21,9 @@ name:            const_i15
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i15
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 15
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 15
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s15) = G_CONSTANT i15 15
     %1:_(s64) = G_ANYEXT %0(s15)
@@ -34,8 +36,9 @@ name:            const_i16
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i16
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 767
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 767
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s16) = G_CONSTANT i16 -64769
     %1:_(s64) = G_ANYEXT %0(s16)
@@ -48,8 +51,9 @@ name:            const_i32
 body:             |
   bb.0.entry:
     ; CHECK-LABEL: name: const_i32
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 -64769
-    ; CHECK-NEXT: $x10 = COPY [[C]](s64)
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 -64769
+    ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[C]](s32)
+    ; CHECK-NEXT: $x10 = COPY [[ANYEXT]](s64)
     ; CHECK-NEXT: PseudoRET implicit $x10
     %0:_(s32) = G_CONSTANT i32 -64769
     %1:_(s64) = G_ANYEXT %0(s32)

arsenm · 2024-02-08T08:06:07Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+  ConstantActions.legalFor({s32, p0}).widenScalarToNextPow2(0).clampScalar(
+      0, s32, sXLen);


should prefer the legal actions first, then custom

arsenm · 2024-02-08T08:07:56Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+  LLT AddrPtrTy = LLT::pointer(AddrSpace, DL.getPointerSizeInBits(AddrSpace));
+  LLT DstLLT = MRI.getType(DstReg);
+
+  Align Alignment = Align(DL.getABITypeAlign(getTypeForLLT(DstLLT, Ctx)));


Don't need the align constructor

arsenm · 2024-02-08T08:08:04Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+  if (!SeqLo.empty() && (SeqLo.size() + 2) <= STI.getMaxBuildIntsCost())
+    return false;
+
+  return true;


return the negated boolean expression?

topperc · 2024-02-09T01:55:19Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+                                                bool shouldOptForSize) const {
+  unsigned BitWidth = APImm.getBitWidth();
+  assert(BitWidth == 32 || BitWidth == 64);
+  uint64_t Imm = APImm.getZExtValue();


I think this should be getSExtValue()? Otherwise the isInt<32> will always fail on BitWidth==32.

yes, my bad, thanks. In non-global version it's also SExt.

topperc · 2024-02-09T01:55:57Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

@@ -451,17 +460,97 @@ bool RISCVLegalizerInfo::legalizeVAStart(MachineInstr &MI,
  return true;
 }

+bool RISCVLegalizerInfo::shouldBeInConstantPool(APInt APImm,
+                                                bool shouldOptForSize) const {


Capitalize variable name.

github-actions · 2024-02-09T05:47:18Z

✅ With the latest revision this PR passed the C/C++ code formatter.

topperc · 2024-02-09T05:49:53Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

@@ -182,7 +185,12 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
    CTPOPActions.maxScalar(0, sXLen).scalarSameSizeAs(1, 0).lower();
  }

-  getActionDefinitionsBuilder({G_CONSTANT, G_IMPLICIT_DEF})
+  auto &ConstantActions = getActionDefinitionsBuilder(G_CONSTANT);
+  ConstantActions.legalFor({s32, p0}) if (ST.is64Bit())


Missing semicolon after the legalFor call?

yes, thanks.

topperc · 2024-02-09T05:50:03Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+                                                bool ShouldOptForSize) const {
+  unsigned BitWidth = APImm.getBitWidth();
+  assert(BitWidth == 32 || BitWidth == 64);
+  uint64_t Imm = APImm.getSExtValue();


mgudim · 2024-02-09T06:22:47Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+}
+
+// TODO: This is almost the same as LegalizerHelper::lowerFConstant and is
+// target-independent. Should we move this to LegalizeHelper?


@topperc Do you think it's a good idea?

Yes, I'm surprised this isn't already implemented there

Sounds like a good idea to me

OK, I'll merge this and make a different PR for the move.

arsenm · 2024-02-09T06:38:41Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+      MF.getConstantPool()->getConstantPoolIndex(ConstVal, Alignment));
+  MachineMemOperand *MMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getConstantPool(MF),
+                              MachineMemOperand::MOLoad, DstLLT, Alignment);


MODereferenceable too

@arsenm How about MOInvariant too?

Aren't those properties inferred from the PseudoSourceValue created from the MachinePointerINfo?

Aren't those properties inferred from the PseudoSourceValue created from the MachinePointerINfo?

Looks like isConstant is:

bool PseudoSourceValue::isConstant(const MachineFrameInfo *) const { if (isStack()) return false; if (isGOT() || isConstantPool() || isJumpTable()) return true; llvm_unreachable("Unknown PseudoSourceValue!"); }

But I don't see anything for Dereferencable. So, should I remove MOInvariant and leave MODereferenceble?

MachineInstr::isDereferenceableInvariantLoad() checks isConstant. Not sure if that's the only place that checks dereferenceable, but it is common one.

It doesn't look like SelectionDAG sets explicitly sets MODereferenceable for constant pool unless its inferred somewhere.

topperc

LGTM

arsenm · 2024-02-09T07:12:18Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+  MachineMemOperand::Flags LoadFlags = MachineMemOperand::MOLoad;
+  LoadFlags |= MachineMemOperand::MODereferenceable;
+  LoadFlags |= MachineMemOperand::MOInvariant;


can set all in the initializer

michaelmaitland · 2024-02-12T20:25:15Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+  LLT AddrPtrTy = LLT::pointer(AddrSpace, DL.getPointerSizeInBits(AddrSpace));
+  LLT DstLLT = MRI.getType(DstReg);
+
+  Align Alignment(DL.getABITypeAlign(getTypeForLLT(DstLLT, Ctx)));


nit: Doesn't getABITypeAlign return an Align? Did we need to create a new Align?

michaelmaitland · 2024-02-12T20:35:16Z

llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp

+  if (ShouldOptForSize)
+    return true;
+  //
+  // Special case. See if we can build the constant as (ADD (SLLI X, C), X) do


Did you test that we build the constant as (ADD (SLLI X, C), X)? What about in the Zba case?

We won't without other changes in the InstructionSelector. We should remove this for now.

mgudim requested review from michaelmaitland, topperc and nitinjohnraj February 8, 2024 07:42

llvmbot added backend:RISC-V llvm:globalisel labels Feb 8, 2024

arsenm reviewed Feb 8, 2024

View reviewed changes

topperc reviewed Feb 9, 2024

View reviewed changes

Addressed review comments.

ae82b9b

topperc reviewed Feb 9, 2024

View reviewed changes

clang-format

4aff721

mgudim force-pushed the gisel-constant-pool branch from cf32cc3 to 4aff721 Compare February 9, 2024 06:12

Added test for constant_pool_i64

8a6ba5a

mgudim commented Feb 9, 2024

View reviewed changes

arsenm reviewed Feb 9, 2024

View reviewed changes

topperc approved these changes Feb 9, 2024

View reviewed changes

Added MODereferenceable and MOInvariant flags.

d5ffbac

arsenm reviewed Feb 9, 2024

View reviewed changes

Removed flags.

0a8e1c8

mgudim merged commit 7192c22 into llvm:main Feb 10, 2024
4 checks passed

michaelmaitland reviewed Feb 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GlobalISel][RISCV] Use constant pool for large integer constants. #81101

[GlobalISel][RISCV] Use constant pool for large integer constants. #81101

mgudim commented Feb 8, 2024

llvmbot commented Feb 8, 2024

llvmbot commented Feb 8, 2024

arsenm Feb 8, 2024

mgudim Feb 9, 2024

arsenm Feb 8, 2024

mgudim Feb 9, 2024

arsenm Feb 8, 2024

mgudim Feb 9, 2024

topperc Feb 9, 2024

mgudim Feb 9, 2024

topperc Feb 9, 2024

mgudim Feb 9, 2024

github-actions bot commented Feb 9, 2024 •

edited

topperc Feb 9, 2024

mgudim Feb 9, 2024

topperc Feb 9, 2024

mgudim Feb 9, 2024

mgudim Feb 9, 2024

arsenm Feb 9, 2024

topperc Feb 9, 2024

mgudim Feb 9, 2024

arsenm Feb 9, 2024

mgudim Feb 9, 2024

arsenm Feb 9, 2024

topperc Feb 9, 2024

mgudim Feb 9, 2024

mgudim Feb 9, 2024

topperc Feb 9, 2024

topperc left a comment

arsenm Feb 9, 2024

michaelmaitland Feb 12, 2024

michaelmaitland Feb 12, 2024

topperc Feb 12, 2024

		ConstantActions.legalFor({s32, p0}).widenScalarToNextPow2(0).clampScalar(
		0, s32, sXLen);

[GlobalISel][RISCV] Use constant pool for large integer constants. #81101

[GlobalISel][RISCV] Use constant pool for large integer constants. #81101

Conversation

mgudim commented Feb 8, 2024

llvmbot commented Feb 8, 2024

llvmbot commented Feb 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 9, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topperc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 9, 2024 •

edited