[AsmPrinter] handles addrspacecast in lowerConstant

[jholewinski]

test/CodeGen/NVPTX/addrspacecast-gvar.ll

6

Shouldn't this be {0, generic(g)+8} ?

[jingyue]

I think you're right. LMTAL why I'm missing the generic.

[jingyue]

This issue turns out more difficult to fix than I expected. With the test case in my patch, LLVM runs into NVPTXAsmPrinter.h:163 and prints out an MCExpr that represents a GEP of an addrspacecast (i32* getelementptr (i32, i32* addrspacecast (i32 addrspace(1)* @g to i32*), i32 2)). Even if we fix lowerConstant to translate addrspacecast into an MCUnaryExpr, printing an MCExpr is not customized yet so we are still unable to print the MCUnaryExpr with generic(.

Can we customize the printing of MCExpr?

[jingyue]

An alternative is to have NVPTXFavorNonGenericAddrSpaces to canonicalize ConstantExpr gep(addrspacecast( to addrspacecast(gep( (this pass currently canonicalizes only Instructions and ConstantExprs that are reachable from load or store), and then NVPTXAsmPrinter can just handle shallower patterns.

[jholewinski]

Handling this aspect of PTX has always been a PITA. Internally, we customize lowerConstant to handle this case and create an MCExpr subclass (NVPTXGenericMCSymbolRefExpr) that handles the printing of "generic(<symbolref>)". Something like this will be needed upstream. Our internal version is a bit of a hack, so I'll want to come up with something cleaner for upstream. If you don't have time to tackle that, I'll try to put something together soon.

[jingyue]

It would be awesome if you can upstream the customized lowerConstant. Thanks in advance!

At the same time, I'd like to try the canonicalization idea I mentioned. I feel this path will be fast and can work around our internal urgent issues. After your upstreaming is done, I'll (partially) revert my canonicalization. How does that sound to you?

[jholewinski]

Sure, go for it! It may be a few days before I get around to looking at this.

[jholewinski]

I committed a fix for this in r236000

[jingyue]

Great! Thanks very much, Justin.

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

const MCExpr *AsmPrinter::lowerConstant(const Constant *CV) {
   }
 
   case Instruction::Trunc:
     // We emit the value and depend on the assembler to truncate the generated
     // expression properly.  This is important for differences between
     // blockaddress labels.  Since the two labels are in the same function, it
     // is reasonable to treat their delta as a 32-bit value.
     // FALL THROUGH.
+  case Instruction::AddrSpaceCast:
   case Instruction::BitCast:
     return lowerConstant(CE->getOperand(0));
 
   case Instruction::IntToPtr: {
     const DataLayout &DL = *TM.getDataLayout();
 
     // Handle casts to pointers by changing them into casts to the appropriate
     // integer type.  This promotes constant folding and simplifies this code.

test/CodeGen/NVPTX/addrspacecast-gvar.ll

 ; RUN: llc < %s -march=nvptx -mcpu=sm_20 | FileCheck %s
 
 ; CHECK: .visible .global .align 4 .u32 g = 42;
 ; CHECK: .visible .global .align 4 .u32 g2 = generic(g);
 ; CHECK: .visible .global .align 4 .u32 g3 = g;
+; CHECK: .visible .global .align 8 .u32 g4[2] = {0, g+8};
 
 @g = addrspace(1) global i32 42
 @g2 = addrspace(1) global i32* addrspacecast (i32 addrspace(1)* @g to i32*)
 @g3 = addrspace(1) global i32 addrspace(1)* @g
+@g4 = constant {i32*, i32*} {i32* null, i32* getelementptr (i32, i32* addrspacecast (i32 addrspace(1)* @g to i32*), i32 2)}