Skip to content
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

[GlobalIsel] Lower integer constants to constant pool in LegalizerHelper. #81957

Merged
merged 4 commits into from
Feb 16, 2024

Conversation

mgudim
Copy link
Contributor

@mgudim mgudim commented Feb 16, 2024

Extend LegalizerHelper's API to lower integer constants to a load from constant pool. Previously, this lowering existed only for FP constants. Apply this change to RISCV.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-llvm-globalisel

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

Author: Mikhail Gudim (mgudim)

Changes

Extend LegalizerHelper's API to lower integer constants to a load from constant pool. Previously, this lowering existed only for FP constants. Apply this change to RISCV.


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

4 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h (+1)
  • (modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+34-13)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp (+1-31)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h (-2)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
index a7ecf0dc1ba216..f001d8a1672972 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
@@ -365,6 +365,7 @@ class LegalizerHelper {
   LegalizeResult bitcastInsertVectorElt(MachineInstr &MI, unsigned TypeIdx,
                                         LLT CastTy);
 
+  LegalizeResult lowerConstant(MachineInstr &MI);
   LegalizeResult lowerFConstant(MachineInstr &MI);
   LegalizeResult lowerBitcast(MachineInstr &MI);
   LegalizeResult lowerLoad(GAnyLoad &MI);
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index 464ff0864d146f..e5450d36cbae51 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -2987,27 +2987,48 @@ static void getUnmergePieces(SmallVectorImpl<Register> &Pieces,
     Pieces.push_back(Unmerge.getReg(I));
 }
 
-LegalizerHelper::LegalizeResult
-LegalizerHelper::lowerFConstant(MachineInstr &MI) {
-  Register Dst = MI.getOperand(0).getReg();
-
+static void emitLoadFromConstantPool(
+    Register DstReg, const Constant *ConstVal,
+    MachineIRBuilder &MIRBuilder) {
+  MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
   MachineFunction &MF = MIRBuilder.getMF();
   const DataLayout &DL = MIRBuilder.getDataLayout();
-
   unsigned AddrSpace = DL.getDefaultGlobalsAddressSpace();
   LLT AddrPtrTy = LLT::pointer(AddrSpace, DL.getPointerSizeInBits(AddrSpace));
-  Align Alignment = Align(DL.getABITypeAlign(
-      getFloatTypeForLLT(MF.getFunction().getContext(), MRI.getType(Dst))));
+  LLT DstLLT = MRI.getType(DstReg);
+
+  Align Alignment(DL.getABITypeAlign(ConstVal->getType()));
 
   auto Addr = MIRBuilder.buildConstantPool(
-      AddrPtrTy, MF.getConstantPool()->getConstantPoolIndex(
-                     MI.getOperand(1).getFPImm(), Alignment));
+      AddrPtrTy,
+      MF.getConstantPool()->getConstantPoolIndex(ConstVal, Alignment));
 
-  MachineMemOperand *MMO = MF.getMachineMemOperand(
-      MachinePointerInfo::getConstantPool(MF), MachineMemOperand::MOLoad,
-      MRI.getType(Dst), Alignment);
+  MachineMemOperand *MMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getConstantPool(MF),
+                              MachineMemOperand::MOLoad, DstLLT, Alignment);
+
+  MIRBuilder.buildLoadInstr(TargetOpcode::G_LOAD, DstReg, Addr, *MMO);
+}
+
+LegalizerHelper::LegalizeResult
+LegalizerHelper::lowerConstant(MachineInstr &MI) {
+  MachineOperand ConstOperand = MI.getOperand(1);
+  assert (ConstOperand.isCImm());
+  const Constant *ConstantVal = ConstOperand.getCImm();
+
+  emitLoadFromConstantPool(MI.getOperand(0).getReg(), ConstantVal, MIRBuilder);
+  MI.eraseFromParent();
+
+  return Legalized;
+}
+
+LegalizerHelper::LegalizeResult
+LegalizerHelper::lowerFConstant(MachineInstr &MI) {
+  MachineOperand ConstOperand = MI.getOperand(1);
+  assert (ConstOperand.isFPImm());
+  const Constant *ConstantVal = ConstOperand.getFPImm();
 
-  MIRBuilder.buildLoadInstr(TargetOpcode::G_LOAD, Dst, Addr, *MMO);
+  emitLoadFromConstantPool(MI.getOperand(0).getReg(), ConstantVal, MIRBuilder);
   MI.eraseFromParent();
 
   return Legalized;
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
index e852052ce5e602..96e74ee486d6af 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
@@ -496,33 +496,6 @@ bool RISCVLegalizerInfo::shouldBeInConstantPool(APInt APImm,
   return !(!SeqLo.empty() && (SeqLo.size() + 2) <= STI.getMaxBuildIntsCost());
 }
 
-// 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(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 {
@@ -544,10 +517,7 @@ bool RISCVLegalizerInfo::legalizeCustom(
     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;
+    return Helper.lowerConstant(MI);
   }
   case TargetOpcode::G_SHL:
   case TargetOpcode::G_ASHR:
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
index 046555f3a3e9ee..323426034827e4 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
+++ b/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.h
@@ -38,8 +38,6 @@ class RISCVLegalizerInfo : public LegalizerInfo {
 
 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;
 

Copy link

github-actions bot commented Feb 16, 2024

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

LegalizerHelper.

Extend LegalizerHelper's API to lower integer constants to a load from
constant pool. Previously, this lowering existed only for FP constants.
Apply this change to RISCV.
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mgudim mgudim merged commit 35cfaec into llvm:main Feb 16, 2024
4 checks passed
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.

None yet

3 participants