[AsmPrinter] Add hook to override constant folding for printing.

[krzysz00]

Summary

Instead of calling ConstantFoldConstant inside the asm printer, use
foldConstantForPrint(), which can be customized per-target.

(Also, fix printing null pointers of size > 64 bits.)

The motivating example for this change is the expression
ptr addrspace(7) addrcast(ptr null to ptr addrspace(7)) on AMDGPU once
address space 7 has been set to an accurate 128-bit length, which is a
change that will occur in a future commit.

The null pointer will be cast to a 128-bit "null descriptor", that is,
i128 0. However, 128-bit values do not fit into MCExpr, and cause
crashes when passed to emitIntValue(). Therefore, we add this constant
folding hook to allow the AMDGPU target to fix up the relevant address
space cast.

[krzysz00]

(Feel free to suggest other reviewers)

[arsenm]

Missing tests

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

3454–3460

Don't understand why you can't unconditionally call emitZeros. Compatibility is not a concern with the codegen APIs

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h

105

I don't understand this hook. Why is printing special? I'd rather extend with an MCBigConstantExpr or similar implementation rather than hack around this

llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp

178

Don't hardcode to generic, check the null pointer representation

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h

105

MCAPIntConstantExpr?

[krzysz00]

Re this being a hack, agreed. I'm open to doing something more principled like you suggested.

On the missing tests front, the problem is that to add a test, I'd need to swap the GCN data layout to include a p7:128:128:128:32 term, which, without a change like this one, causes the existing tests to crash.

Would it be more appropriate to make the data layout change first and then XFAIL (or comment out the relevant parts of) nulls.ll, to land that first, to land this despite a general lack of tests, or to put this and the data layout changes in one larger commit?

[arsenm]

In D143515#4110761, @krzysz00 wrote:

Re this being a hack, agreed. I'm open to doing something more principled like you suggested.

On the missing tests front, the problem is that to add a test, I'd need to swap the GCN data layout to include a p7:128:128:128:32 term, which, without a change like this one, causes the existing tests to crash.

Would it be more appropriate to make the data layout change first and then XFAIL (or comment out the relevant parts of) nulls.ll, to land that first, to land this despite a general lack of tests, or to put this and the data layout changes in one larger commit?

Why would they crash? Just make the datalayout change first, there's little reason to put that one off

[krzysz00]

The crash is because of emitIntValue() having an assert(Size <= 8) in it

[MaskRay]

A bit strange but I think it is fine to have the hook if AMDGPU really needs it.

[arsenm]

In D143515#4110929, @krzysz00 wrote:

The crash is because of emitIntValue() having an assert(Size <= 8) in it

I mean why do we have emitIntValue at all. Why isn't emitZeros always used?

[krzysz00]

@MaskRay Yeah, this is strange, and I'm with @arsenm on needing to find a less ugly solution to this problem.

Re emitZeros: I suspect there may be some reason to use, say, .quad 0 or .long 0 over .zeroes N, and that difference might actually matter when you're dealing with debug info or something? I'm not sure, but I didn't want to upset the structures.

For context, the call chain that hits the bug is as follows

1. The problematic expression (addrspace cast ptr null to addrspace(7)) hits emitGlobalConstantImpl
2. There, it's not recoginized as any of the cases that emitGlobalConstantImpl knows how to handle by itself, so it calls lowerConstant to produce an MCExpr from said expression
3. We step in and do the "adddrspace cast of null pointers" handling, which, in this case, would just return a 0, and we correctly tag that 0 with a size of 16 bytes/128 bits.
4. Said 0, being an MCExpr, gets sent to emitValue(), which sends it to emitIntValue().

4b. That MCExpr holds a uint64_t, not an APInt, so none of the MCExpr code is prepared to deal with values over 64 bits.

5. emitIntValue() sensibly asserts that the value it's being asked to emit is 8 bytes or less, which we just broke

My hack was to provide a pathway for targets to send a value back to the start of emitGlobalConstantImpl(), but that doesn't really solve the problem.

[nikic]

(I don't have the necessary MC / AsmPrinter context here)

[arsenm]

I think adding a big-int MCExpr would not be difficult and would avoid the need for this hack to specially handle 0

[krzysz00]

Having done some looking, I don't think that it's trivial to add bigint support to MCExpr, since that would mean that the MC emitting infrastructure would need to know the target endianness, which it doesn't look like it does.

And there's a lot of functions on MCExpr that return [u]int64_t, and we'd either need those to fail on big ints or we'd need to rework them to use APInts or something.

[arsenm]

In D143515#4132519, @krzysz00 wrote:

Having done some looking, I don't think that it's trivial to add bigint support to MCExpr, since that would mean that the MC emitting infrastructure would need to know the target endianness, which it doesn't look like it does.

This doesn't make any sense, of course it has to know the endianness? The endianness is the most global of flags in the TM and DataLayout

llvm/include/llvm/CodeGen/AsmPrinter.h

public:
 
   /// Emit an alignment directive to the specified power of two boundary. If a
   /// global value is specified, and if that global has an explicit alignment
   /// requested, it will override the alignment request if required for
   /// correctness.
   void emitAlignment(Align Alignment, const GlobalObject *GV = nullptr,
                      unsigned MaxBytesToEmit = 0) const;
 
+  /// Constant-fold Constant as neened to allow it to be emitted into assembly.
+  /// By default, this calls ConstantFoldConstant(), which should be sufficient,
+  /// but this hook is provided to allow targets to define additional constant
+  /// folding rules that cannot be defined in `lowerConstant`. For example,
+  /// on AMDGPU, casting the null pointer to addrspace(7) yields a 128-bit
+  /// null descriptor that can't fit into MCExpr.
+  virtual Constant *constantFoldForPrint(const Constant *CV);
+
   /// Lower the specified LLVM Constant to an MCExpr.
   virtual const MCExpr *lowerConstant(const Constant *CV);
 
   /// Print a general LLVM constant to the .s file.
   /// On AIX, when an alias refers to a sub-element of a global variable, the
   /// label of that alias needs to be emitted before the corresponding element.
   using AliasMapTy = DenseMap<uint64_t, SmallVector<const GlobalAlias *, 1>>;
   void emitGlobalConstant(const DataLayout &DL, const Constant *CV,

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::emitAlignment(Align Alignment, const GlobalObject *GV,
   } else
     OutStreamer->emitValueToAlignment(Alignment, 0, 1, MaxBytesToEmit);
 }
 
 //===----------------------------------------------------------------------===//
 // Constant emission.
 //===----------------------------------------------------------------------===//
 
+Constant *AsmPrinter::constantFoldForPrint(const Constant *CV) {
+  return ConstantFoldConstant(CV, getDataLayout());
+}
+
 const MCExpr *AsmPrinter::lowerConstant(const Constant *CV) {
   MCContext &Ctx = OutContext;
 
   if (CV->isNullValue() || isa<UndefValue>(CV))
     return MCConstantExpr::create(0, Ctx);
 
   if (const ConstantInt *CI = dyn_cast<ConstantInt>(CV))
     return MCConstantExpr::create(CI->getZExtValue(), Ctx);
case Instruction::Add: {
     const MCExpr *RHS = lowerConstant(CE->getOperand(1));
     return MCBinaryExpr::createAdd(LHS, RHS, Ctx);
   }
   }
 
   // If the code isn't optimized, there may be outstanding folding
   // opportunities. Attempt to fold the expression using DataLayout as a
   // last resort before giving up.
-  Constant *C = ConstantFoldConstant(CE, getDataLayout());
+  Constant *C = constantFoldForPrint(CE);
   if (C != CE)
     return lowerConstant(C);
 
   // Otherwise report the problem to the user.
   std::string S;
   raw_string_ostream OS(S);
   OS << "Unsupported expression in static initializer: ";
   CE->printAsOperand(OS, /*PrintType=*/false,
if (const ConstantInt *CI = dyn_cast<ConstantInt>(CV)) {
 
     return;
   }
 
   if (const ConstantFP *CFP = dyn_cast<ConstantFP>(CV))
     return emitGlobalConstantFP(CFP, AP);
 
   if (isa<ConstantPointerNull>(CV)) {
+    if (Size > 8) {
+      // Not always used in order to preserve backwards compatibility while
+      // allowing for large pointers.
+      AP.OutStreamer->emitZeros(Size);
+      return;
+    }
     AP.OutStreamer->emitIntValue(0, Size);
     return;
   }
 
   if (const ConstantDataSequential *CDS = dyn_cast<ConstantDataSequential>(CV))
     return emitGlobalConstantDataSequential(DL, CDS, AP, AliasList);
 
   if (const ConstantArray *CVA = dyn_cast<ConstantArray>(CV))
     return emitGlobalConstantArray(DL, CVA, AP, BaseCV, Offset, AliasList);
 
   if (const ConstantStruct *CVS = dyn_cast<ConstantStruct>(CV))
     return emitGlobalConstantStruct(DL, CVS, AP, BaseCV, Offset, AliasList);
 
   if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(CV)) {
     // Look through bitcasts, which might not be able to be MCExpr'ized (e.g. of
     // vectors).
     if (CE->getOpcode() == Instruction::BitCast)
       return emitGlobalConstantImpl(DL, CE->getOperand(0), AP);
 
     if (Size > 8) {
       // If the constant expression's size is greater than 64-bits, then we have
       // to emit the value in chunks. Try to constant fold the value and emit it
       // that way.
-      Constant *New = ConstantFoldConstant(CE, DL);
+      Constant *New = AP.constantFoldForPrint(CE);
       if (New != CE)
         return emitGlobalConstantImpl(DL, New, AP);
     }
   }
 
   if (const ConstantVector *V = dyn_cast<ConstantVector>(CV))
     return emitGlobalConstantVector(DL, V, AP, AliasList);
 

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h

public:
   bool doInitialization(Module &M) override;
   bool doFinalization(Module &M) override;
   bool runOnMachineFunction(MachineFunction &MF) override;
 
   /// Wrapper for MCInstLowering.lowerOperand() for the tblgen'erated
   /// pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const;
 
+  /// Lower a cast of the null pointer to address space 7, which produces a
+  /// 128-bit null buffer descriptor, to a null constant value, because 128-bit
+  /// values do not fit in MCExpr.
+  Constant *constantFoldForPrint(const Constant *CV) override;
+
   /// Lower the specified LLVM Constant to an MCExpr.
   /// The AsmPrinter::lowerConstantof does not know how to lower
   /// addrspacecast, therefore they should be lowered by this function.
   const MCExpr *lowerConstant(const Constant *CV) override;
 
   /// tblgen'erated driver function for lowering simple MI->MC pseudo
   /// instructions.
   bool emitPseudoExpansionLowering(MCStreamer &OutStreamer,

llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp

 
 bool AMDGPUAsmPrinter::lowerOperand(const MachineOperand &MO,
                                     MCOperand &MCOp) const {
   const GCNSubtarget &STI = MF->getSubtarget<GCNSubtarget>();
   AMDGPUMCInstLower MCInstLowering(OutContext, STI, *this);
   return MCInstLowering.lowerOperand(MO, MCOp);
 }
 
+Constant *AMDGPUAsmPrinter::constantFoldForPrint(const Constant *CV) {
+  // TargetMachine does not support llvm-style cast. Use C++-style cast.
+  // This is safe since TM is always of type AMDGPUTargetMachine or its
+  // derived class.
+  auto &AT = static_cast<const AMDGPUTargetMachine &>(TM);
+  auto *CE = dyn_cast<ConstantExpr>(CV);
+  if (CE && CE->getOpcode() == Instruction::AddrSpaceCast) {
+    auto *Op = CE->getOperand(0);
+    auto SrcAddr = Op->getType()->getPointerAddressSpace();
+    auto DstAddr = CE->getType()->getPointerAddressSpace();
+    if (Op->isNullValue() && AT.getNullPointerValue(SrcAddr) == 0 &&
+        AT.getPointerSizeInBits(DstAddr) > 64)
+      return ConstantPointerNull::get(cast<PointerType>(CE->getType()));
+  }
+  return AsmPrinter::constantFoldForPrint(CV);
+}
+
 const MCExpr *AMDGPUAsmPrinter::lowerConstant(const Constant *CV) {
 
   // Intercept LDS variables with known addresses
   if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(CV)) {
     if (AMDGPUMachineFunction::isKnownAddressLDSGlobal(*GV)) {
       unsigned offset =
           AMDGPUMachineFunction::calculateKnownAddressOfLDSGlobal(*GV);
       Constant *C = ConstantInt::get(CV->getContext(), APInt(32, offset));