[AsmPrinter] Fix emission of non-standard integer constants for BE targets

[LemonBoy]

Summary

The code assumed that zero-extending the integer constant to the designated alloc size would be fine even for BE targets, but that's not the case as that pulls in zeros from the MSB side while we actually expect the padding zeros to go after the LSB.

I've changed the codepath handling the constant integers to use the store size for both small(er than u64) and big constants and then add zero padding right after that.

[qcolombet]

Nice catch!

The new assembly makes sense to me. Thanks for fixing it!

[LemonBoy]

Can you (or anyone) commit this on my behalf? I don't have commit access.

[LemonBoy]

Ping?

[efriedma]

How do you want to be credited in the "author" line of the git commit (name+email)?

[LemonBoy]

Yeah, that's fine

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

if (ExtraBitsSize) {
     // * Record the extra bits to emit.
     // * Realign the raw data to emit the chunks of 64-bits.
     if (DL.isBigEndian()) {
       // Basically the structure of the raw data is a chunk of 64-bits cells:
       //    0        1         BitWidth / 64
       // [chunk1][chunk2] ... [chunkN].
       // The most significant chunk is chunkN and it should be emitted first.
       // However, due to the alignment issue chunkN contains useless bits.
-      // Realign the chunks so that they contain only useless information:
+      // Realign the chunks so that they contain only useful information:
       // ExtraBits     0       1       (BitWidth / 64) - 1
       //       chu[nk1 chu][nk2 chu] ... [nkN-1 chunkN]
+      ExtraBitsSize = alignTo(ExtraBitsSize, 8);
       ExtraBits = Realigned.getRawData()[0] &
         (((uint64_t)-1) >> (64 - ExtraBitsSize));
       Realigned.lshrInPlace(ExtraBitsSize);
     } else
       ExtraBits = Realigned.getRawData()[BitWidth / 64];
   }
 
   // We don't expect assemblers to support integer data directives
   // for more than 64 bits, so we emit the data in at most 64-bit
   // quantities at a time.
   const uint64_t *RawData = Realigned.getRawData();
   for (unsigned i = 0, e = BitWidth / 64; i != e; ++i) {
     uint64_t Val = DL.isBigEndian() ? RawData[e - i - 1] : RawData[i];
     AP.OutStreamer->emitIntValue(Val, 8);
   }
 
   if (ExtraBitsSize) {
     // Emit the extra bits after the 64-bits chunks.
 
     // Emit a directive that fills the expected size.
-    uint64_t Size = AP.getDataLayout().getTypeAllocSize(CI->getType());
+    uint64_t Size = AP.getDataLayout().getTypeStoreSize(CI->getType());
     Size -= (BitWidth / 64) * 8;
     assert(Size && Size * 8 >= ExtraBitsSize &&
            (ExtraBits & (((uint64_t)-1) >> (64 - ExtraBitsSize)))
            == ExtraBits && "Directive too small for extra bits.");
     AP.OutStreamer->emitIntValue(ExtraBits, Size);
   }
 }
 
static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
   // constant symbol base and the current position with BaseCV and Offset.
   if (!BaseCV && CV->hasOneUse())
     BaseCV = dyn_cast<Constant>(CV->user_back());
 
   if (isa<ConstantAggregateZero>(CV) || isa<UndefValue>(CV))
     return AP.OutStreamer->emitZeros(Size);
 
   if (const ConstantInt *CI = dyn_cast<ConstantInt>(CV)) {
-    switch (Size) {
-    case 1:
-    case 2:
-    case 4:
-    case 8:
+    const uint64_t StoreSize = DL.getTypeStoreSize(CV->getType());
+
+    if (StoreSize < 8) {
       if (AP.isVerbose())
         AP.OutStreamer->GetCommentOS() << format("0x%" PRIx64 "\n",
                                                  CI->getZExtValue());
-      AP.OutStreamer->emitIntValue(CI->getZExtValue(), Size);
-      return;
-    default:
+      AP.OutStreamer->emitIntValue(CI->getZExtValue(), StoreSize);
+    } else {
       emitGlobalConstantLargeInt(CI, AP);
-      return;
     }
+
+    // Emit tail padding if needed
+    if (Size != StoreSize)
+      AP.OutStreamer->emitZeros(Size - StoreSize);
+
+    return;
   }
 
   if (const ConstantFP *CFP = dyn_cast<ConstantFP>(CV))
     return emitGlobalConstantFP(CFP, AP);
 
   if (isa<ConstantPointerNull>(CV)) {
     AP.OutStreamer->emitIntValue(0, Size);
     return;

llvm/test/CodeGen/ARM/emit-big-cst.ll

 ; RUN: llc -mtriple=thumbv7-unknown-unknown -target-abi apcs < %s | FileCheck %s
 ; Check assembly printing of odd constants.
 
 ; CHECK: bigCst:
 ; CHECK-NEXT: .long 1694510592
 ; CHECK-NEXT: .long 2960197
-; CHECK-NEXT: .long 26220
+; CHECK-NEXT: .short 26220
+; CHECK-NEXT: .byte 0
+; CHECK-NEXT: .zero 1
 ; CHECK-NEXT: .size bigCst, 12
 
 @bigCst = internal constant i82 483673642326615442599424
 
 define void @accessBig(i64* %storage) {
   %addr = bitcast i64* %storage to i82*
   %bigLoadedCst = load volatile i82, i82* @bigCst
   %tmp = add i82 %bigLoadedCst, 1
   store i82 %tmp, i82* %addr
   ret void
 }

llvm/test/CodeGen/Mips/emit-big-cst.ll

-; RUN: llc -march=mips < %s | FileCheck %s
+; RUN: llc -march=mips < %s | FileCheck %s --check-prefix=BE
+; RUN: llc -march=mipsel < %s | FileCheck %s --check-prefix=LE
 ; Check assembly printing of odd constants.
 
-; CHECK: bigCst:
-; CHECK-NEXT: .8byte 1845068520838224192
-; CHECK-NEXT: .8byte 11776
-; CHECK-NEXT: .size bigCst, 16
+; BE-LABEL: bigCst:
+; BE-NEXT: .8byte 28829195638097253
+; BE-NEXT: .2byte 46
+; BE-NEXT: .byte 0
+; BE-NEXT: .space 5
+; BE-NEXT: .size bigCst, 16
+
+; LE-LABEL: bigCst:
+; LE-NEXT: .8byte 12713950999227904
+; LE-NEXT: .2byte 26220
+; LE-NEXT: .byte 0
+; LE-NEXT: .space 5
+; LE-NEXT: .size bigCst, 16
+
+; BE-LABEL: smallCst:
+; BE-NEXT: .2byte 4386
+; BE-NEXT: .byte 51
+; BE-NEXT: .space 1
+; BE-NEXT: .size smallCst, 4
+
+; LE-LABEL: smallCst:
+; LE-NEXT: .2byte 8755
+; LE-NEXT: .byte 17
+; LE-NEXT: .space 1
+; LE-NEXT: .size smallCst, 4
 
 @bigCst = internal constant i82 483673642326615442599424
 
 define void @accessBig(i64* %storage) {
   %addr = bitcast i64* %storage to i82*
   %bigLoadedCst = load volatile i82, i82* @bigCst
   %tmp = add i82 %bigLoadedCst, 1
   store i82 %tmp, i82* %addr
   ret void
 }
+
+@smallCst = internal constant i24 1122867

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

 define i32 @main() #0 {
 entry:
   ret i32 0
 }
 
 ;CHECK:         .csect .rodata[RO]
 ;CHECK-NEXT:         .align  4
 ;CHECK-NEXT: .L__const.main.cnst32:
-;CHECK-NEXT:         .llong  4611686018427387954     # 0x4000000000000032
+;CHECK-NEXT:         .llong  4611686018427387954
 ;CHECK-NEXT:         .long   0                       # 0x0
 ;CHECK-NEXT:         .space  4
-;CHECK-NEXT:         .llong  0                       # 0x0
+;CHECK-NEXT:         .llong  0
 ;CHECK-NEXT:         .long   0                       # 0x0
 ;CHECK-NEXT:         .space  4
 ;CHECK-NEXT:         .align  3
 ;CHECK-NEXT: .L__const.main.cnst16:
-;CHECK-NEXT:         .llong  4611686018427387926     # 0x4000000000000016
+;CHECK-NEXT:         .llong  4611686018427387926
 ;CHECK-NEXT:         .long   0                       # 0x0
 ;CHECK-NEXT:         .space  4
 ;CHECK-NEXT:         .align  3
 ;CHECK-NEXT: .L__const.main.cnst8:
 ;CHECK-NEXT:         .long   1073741832              # 0x40000008
 ;CHECK-NEXT:         .long   0                       # 0x0
 ;CHECK-NEXT:         .align  3
 ;CHECK-NEXT: .L__const.main.cnst4:

llvm/test/CodeGen/X86/emit-big-cst.ll

 ; RUN: llc -mtriple=x86_64-unknown-unknown < %s | FileCheck %s
 ; Check assembly printing of odd constants.
 
 ; CHECK: bigCst:
 ; CHECK-NEXT: .quad 12713950999227904
-; CHECK-NEXT: .quad 26220
+; CHECK-NEXT: .short 26220
+; CHECK-NEXT: .byte 0
+; CHECK-NEXT: .zero 5
 ; CHECK-NEXT: .size bigCst, 16
 
 @bigCst = internal constant i82 483673642326615442599424
 
 define void @accessBig(i64* %storage) {
   %addr = bitcast i64* %storage to i82*
   %bigLoadedCst = load volatile i82, i82* @bigCst
   %tmp = add i82 %bigLoadedCst, 1
   store i82 %tmp, i82* %addr
   ret void
 }

llvm/test/CodeGen/X86/global-fill.ll

 ; RUN: llc -mtriple=x86_64-apple-darwin < %s | FileCheck %s
 
 @test1 = global [2 x i24] [i24 -1, i24 -1]
 ; CHECK-LABEL: test1:
-; CHECK-NEXT: .long 16777215
-; CHECK-NEXT: .long 16777215
+; CHECK-NEXT: .short  65535
+; CHECK-NEXT: .byte 255
+; CHECK-NEXT: .space  1
+; CHECK-NEXT: .short  65535
+; CHECK-NEXT: .byte 255
+; CHECK-NEXT: .space  1
 
 @test2 = global [2 x i7] [i7 1, i7 1]
 ; CHECK-LABEL: test2:
 ; CHECK-NEXT: .space 2,1
 
 @test3 = global [4 x i128] [i128 -1, i128 -1, i128 -1, i128 -1]
 ; CHECK-LABEL: test3:
 ; CHECK-NEXT: .space 64,255