[NFC] Add the getSizeInBytes() interface for MachineConstantPoolValue

[steven.zhang]

Summary

Current implementation assumes that, each MachineConstantPoolValue takes up sizeof(MachineConstantPoolValue::Ty) bytes. For PowerPC, we want to lump all the constants with the same type as one MachineConstantPoolValue to save the cost that calculate the TOC entry for each const. So, we need to extend the MachineConstantPoolValue that break this assumption.
That is, transform:

addis 3, 2, .LCPI0_0@toc@ha
lfd 0, .LCPI0_0@toc@l(3)
addis 3, 2, .LCPI0_1@toc@ha
lfd 1, .LCPI0_1@toc@l(3)

to:

addis 3, 2, .LC0@toc@ha
ld 3, .LC0@toc@l(3)
lfd 0, 8(3)
lfd 1, 0(3)

And

.LCPI0_0:
        .quad   0x40218d4fdf3b645a              # double 8.7759999999999998
.LCPI0_1:
        .quad   0x49818d4fdf3b645a              # double 1.2525525751187504E+46

->

.LCPI0_0:
        .quad   0x49818d4fdf3b645a              # double 1.2525525751187504E+46
        .quad   0x40218d4fdf3b645a              # double 8.7759999999999998
        .section        ".note.GNU-stack","",@progbits
        .section        .toc,"aw",@progbits
.LC0:
        .tc .LCPI0_0[TC],.LCPI0_0

The more constants accessed, the more benefit we will have.

And this assumption also doesn't make sense to me as we have already provided mechanism for target to customize the constant pool with MachineConstantPoolValue. We shouldn't assume its size.

[steven.zhang]

llvm/lib/Target/X86/X86MCInstLower.cpp

1788

The assertion here won't help too much as ConstantEntry.getType() is essential C->getType(). Remove this assertion to remove the getType() from MachineConstantPoolEntry.

[MaskRay]

TOC is a step forward from GOT for which one can control the layout. This kind of optimizations should be done (otherwise we can generate @got in the first place).

llvm/include/llvm/CodeGen/MachineConstantPool.h

46

Name - is deprecated in the coding standard. Please drop.

Actually the function name self explains so perhaps the comment can be omitted.

[steven.zhang]

Address comments.

[MaskRay]

Can you please post the dependent patch to make it clear whether this refactoring is suitable?

llvm/lib/Target/ARM/ARMConstantIslandPass.cpp

517

if const unsigned works, please change as well.

[steven.zhang]

In D89108#2323163, @MaskRay wrote:

Can you please post the dependent patch to make it clear whether this refactoring is suitable?

Ok, I will post the dependent patches later.

llvm/lib/Target/ARM/ARMConstantIslandPass.cpp

517

Sorry, do you mean const unsigned Size = ... ? I don't think there is any benefit to have the cv-qualifier here but increasing the code size.

[steven.zhang]

https://reviews.llvm.org/D91053 is posted for the full picture.

[RKSimon]

llvm/include/llvm/CodeGen/MachineConstantPool.h

46

Very pendantic - but should we name this getSizeInBytes()?

[steven.zhang]

Thank you for the review and Merry Christmas !

Address comments.

[RKSimon]

LGTM

llvm/include/llvm/CodeGen/MachineConstantPool.h

class MachineConstantPoolValue {
   virtual void anchor();
 
   Type *Ty;
 
 public:
   explicit MachineConstantPoolValue(Type *ty) : Ty(ty) {}
   virtual ~MachineConstantPoolValue() = default;
 
-  /// getType - get type of this MachineConstantPoolValue.
-  ///
   Type *getType() const { return Ty; }
 
+  virtual unsigned getSizeInBytes(const DataLayout &DL) const;
+
   virtual int getExistingMachineCPValue(MachineConstantPool *CP,
                                         Align Alignment) = 0;
 
   virtual void addSelectionDAGCSEId(FoldingSetNodeID &ID) = 0;
 
   /// print - Implement operator<<
   virtual void print(raw_ostream &O) const = 0;
 };
public:
 
   /// isMachineConstantPoolEntry - Return true if the MachineConstantPoolEntry
   /// is indeed a target specific constantpool entry, not a wrapper over a
   /// Constant.
   bool isMachineConstantPoolEntry() const { return IsMachineConstantPoolEntry; }
 
   Align getAlign() const { return Alignment; }
 
-  Type *getType() const;
+  unsigned getSizeInBytes(const DataLayout &DL) const;
 
   /// This method classifies the entry according to whether or not it may
   /// generate a relocation entry.  This must be conservative, so if it might
   /// codegen to a relocatable entry, it should say so.
   bool needsRelocation() const;
 
   SectionKind getSectionKind(const DataLayout *DL) const;
 };

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

for (unsigned j = 0, ee = CPSections[i].CPEs.size(); j != ee; ++j) {
       }
 
       MachineConstantPoolEntry CPE = CP[CPI];
 
       // Emit inter-object padding for alignment.
       unsigned NewOffset = alignTo(Offset, CPE.getAlign());
       OutStreamer->emitZeros(NewOffset - Offset);
 
-      Type *Ty = CPE.getType();
-      Offset = NewOffset + getDataLayout().getTypeAllocSize(Ty);
+      Offset = NewOffset + CPE.getSizeInBytes(getDataLayout());
 
       OutStreamer->emitLabel(Sym);
       if (CPE.isMachineConstantPoolEntry())
         emitMachineConstantPoolValue(CPE.Val.MachineCPVal);
       else
         emitGlobalConstant(getDataLayout(), CPE.Val.ConstVal);
     }
   }

llvm/lib/CodeGen/MachineFunction.cpp

 }
 
 //===----------------------------------------------------------------------===//
 //  MachineConstantPool implementation
 //===----------------------------------------------------------------------===//
 
 void MachineConstantPoolValue::anchor() {}
 
-Type *MachineConstantPoolEntry::getType() const {
+unsigned MachineConstantPoolValue::getSizeInBytes(const DataLayout &DL) const {
+  return DL.getTypeAllocSize(Ty);
+}
+
+unsigned MachineConstantPoolEntry::getSizeInBytes(const DataLayout &DL) const {
   if (isMachineConstantPoolEntry())
-    return Val.MachineCPVal->getType();
-  return Val.ConstVal->getType();
+    return Val.MachineCPVal->getSizeInBytes(DL);
+  return DL.getTypeAllocSize(Val.ConstVal->getType());
 }
 
 bool MachineConstantPoolEntry::needsRelocation() const {
   if (isMachineConstantPoolEntry())
     return true;
   return Val.ConstVal->needsRelocation();
 }
 
 SectionKind
 MachineConstantPoolEntry::getSectionKind(const DataLayout *DL) const {
   if (needsRelocation())
     return SectionKind::getReadOnlyWithRel();
-  switch (DL->getTypeAllocSize(getType())) {
+  switch (getSizeInBytes(*DL)) {
   case 4:
     return SectionKind::getMergeableConst4();
   case 8:
     return SectionKind::getMergeableConst8();
   case 16:
     return SectionKind::getMergeableConst16();
   case 32:
     return SectionKind::getMergeableConst32();

llvm/lib/Target/ARM/ARMConstantIslandPass.cpp

SmallVector<MachineBasicBlock::iterator, 8> InsPoint(MaxLogAlign + 1,
                                                        BB->end());
 
   // Add all of the constants from the constant pool to the end block, use an
   // identity mapping of CPI's to CPE's.
   const std::vector<MachineConstantPoolEntry> &CPs = MCP->getConstants();
 
   const DataLayout &TD = MF->getDataLayout();
   for (unsigned i = 0, e = CPs.size(); i != e; ++i) {
-    unsigned Size = TD.getTypeAllocSize(CPs[i].getType());
+    unsigned Size = CPs[i].getSizeInBytes(TD);
     Align Alignment = CPs[i].getAlign();
     // Verify that all constant pool entries are a multiple of their alignment.
     // If not, we would have to pad them out so that instructions stay aligned.
     assert(isAligned(Alignment, Size) && "CP Entry not multiple of 4 bytes!");
 
     // Insert CONSTPOOL_ENTRY before entries with a smaller alignment.
     unsigned LogAlign = Log2(Alignment);
     MachineBasicBlock::iterator InsAt = InsPoint[LogAlign];

llvm/lib/Target/Mips/MipsConstantIslandPass.cpp

SmallVector<MachineBasicBlock::iterator, 8> InsPoint(Log2(MaxAlign) + 1,
                                                        BB->end());
 
   // Add all of the constants from the constant pool to the end block, use an
   // identity mapping of CPI's to CPE's.
   const std::vector<MachineConstantPoolEntry> &CPs = MCP->getConstants();
 
   const DataLayout &TD = MF->getDataLayout();
   for (unsigned i = 0, e = CPs.size(); i != e; ++i) {
-    unsigned Size = TD.getTypeAllocSize(CPs[i].getType());
+    unsigned Size = CPs[i].getSizeInBytes(TD);
     assert(Size >= 4 && "Too small constant pool entry");
     Align Alignment = CPs[i].getAlign();
     // Verify that all constant pool entries are a multiple of their alignment.
     // If not, we would have to pad them out so that instructions stay aligned.
     assert(isAligned(Alignment, Size) && "CP Entry not multiple of 4 bytes!");
 
     // Insert CONSTPOOL_ENTRY before entries with a smaller alignment.
     unsigned LogAlign = Log2(Alignment);

llvm/lib/Target/X86/X86MCInstLower.cpp

ArrayRef<MachineConstantPoolEntry> Constants =
       MI.getParent()->getParent()->getConstantPool()->getConstants();
   const MachineConstantPoolEntry &ConstantEntry = Constants[Op.getIndex()];
 
   // Bail if this is a machine constant pool entry, we won't be able to dig out
   // anything useful.
   if (ConstantEntry.isMachineConstantPoolEntry())
     return nullptr;
 
-  const Constant *C = ConstantEntry.Val.ConstVal;
-  assert((!C || ConstantEntry.getType() == C->getType()) &&
-         "Expected a constant of the same type!");
-  return C;
+  return ConstantEntry.Val.ConstVal;
 }
 
 static std::string getShuffleComment(const MachineInstr *MI, unsigned SrcOp1Idx,
                                      unsigned SrcOp2Idx, ArrayRef<int> Mask) {
   std::string Comment;
 
   // Compute the name for a register. This is really goofy because we have
   // multiple instruction printers that could (in theory) use different