[ARM] handle promotion of zero sized constants.

[TimNN]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=32130

The special case of zero sized values was previously not handled correctly. This patch handles this by not promoting if the size is zero.

[jmolloy]

Hi Tim,

Thanks for looking into this. I'm not 100% certain about your patch though. The intent in this code is that everything gets padded to a multiple of 4 bytes (see PaddingPossible and RequiredPadding above). I'd expect that in the case you describe, RequiredPadding would be != 4, so the loop at line 3095 would padd out the global and avoid putting too-small data in the constant pool.

Without a reduced testcase (which would be needed to commit the change) I can't offhand see where the problem could actually be.

Cheers,

James

[jmolloy]

Hi Tim,

I see, thanks. Yes, you're right, the abnormal case of Size == 0 would indeed blow this code up. Can I suggest:

1. You add a "|| Size == 0" term to the if condition on line 3066
2. You add a testcase.

The testcase should be a LIT test. You can see examples in the test/CodeGen/ARM directory which should be where this test should go. Have a look at test/CodeGen/ARM/constantpool-promote.ll - you can tag your test onto the end there.

Something like (appending after the @test9 function):

@zerosize = private unnamed_addr constant [0 x i16] zeroinitializer, align 4
;; Comment for this test
; CHECK-LABEL: @pr32130
; CHECK: -NOT: adr
define void @pr32130() #0 {
  tail call void @c(i16* getelementptr inbounds ([0 x i16], [0 x i16]* @zerosize, i32 0, i32 0)) #2
  ret void
}

[TimNN]

I have updated the patch as suggested.

[jmolloy]

LGTM, thanks!

[TimNN]

I cannot commit this myself, could someone please do it for me?

lib/Target/ARM/ARMISelLowering.cpp

static SDValue promoteToConstantPool(const GlobalValue *GV, SelectionDAG &DAG,
   // are paddable to a multiple of 4. Currently we only try and pad constants
   // that are strings for simplicity.
   auto *CDAInit = dyn_cast<ConstantDataArray>(Init);
   unsigned Size = DAG.getDataLayout().getTypeAllocSize(Init->getType());
   unsigned Align = GVar->getAlignment();
   unsigned RequiredPadding = 4 - (Size % 4);
   bool PaddingPossible =
     RequiredPadding == 4 || (CDAInit && CDAInit->isString());
-  if (!PaddingPossible || Align > 4 || Size > ConstpoolPromotionMaxSize)
+  if (!PaddingPossible || Align > 4 || Size > ConstpoolPromotionMaxSize ||
+      Size == 0)
     return SDValue();
 
   unsigned PaddedSize = Size + ((RequiredPadding == 4) ? 0 : RequiredPadding);
   MachineFunction &MF = DAG.getMachineFunction();
   ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
 
   // We can't bloat the constant pool too much, else the ConstantIslands pass
   // may fail to converge. If we haven't promoted this global yet (it may have

test/CodeGen/ARM/constantpool-promote.ll

 @.str2 = private unnamed_addr constant [27 x i8] c"this string is just right!\00", align 1
 @.str3 = private unnamed_addr constant [26 x i8] c"this string is used twice\00", align 1
 @.str4 = private unnamed_addr constant [29 x i8] c"same string in two functions\00", align 1
 @.arr1 = private unnamed_addr constant [2 x i16] [i16 3, i16 4], align 2
 @.arr2 = private unnamed_addr constant [2 x i16] [i16 7, i16 8], align 2
 @.arr3 = private unnamed_addr constant [2 x i16*] [i16* null, i16* null], align 4
 @.ptr = private unnamed_addr constant [2 x i16*] [i16* getelementptr inbounds ([2 x i16], [2 x i16]* @.arr2, i32 0, i32 0), i16* null], align 2
 @.arr4 = private unnamed_addr constant [2 x i16] [i16 3, i16 4], align 16
+@.zerosize = private unnamed_addr constant [0 x i16] zeroinitializer, align 4
 
 ; CHECK-LABEL: @test1
 ; CHECK: adr r0, [[x:.*]]
 ; CHECK: [[x]]:
 ; CHECK: .asciz "s\000\000"
 define void @test1() #0 {
   tail call void @a(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i32 0, i32 0)) #2
   ret void

 ; This shouldn't be promoted, as the global requires >4 byte alignment.
 ; CHECK-LABEL: @test9
 ; CHECK-NOT: adr
 define void @test9() #0 {
   tail call void @c(i16* getelementptr inbounds ([2 x i16], [2 x i16]* @.arr4, i32 0, i32 0)) #2
   ret void
 }
 
+; Ensure that zero sized values are supported / not promoted.
+; CHECK-LABEL: @pr32130
+; CHECK-NOT: adr
+define void @pr32130() #0 {
+  tail call void @c(i16* getelementptr inbounds ([0 x i16], [0 x i16]* @.zerosize, i32 0, i32 0)) #2
+  ret void
+}
+
 ; CHECK-LABEL: @test10
 ; CHECK-V6M: adr r{{[0-9]*}}, [[x:.*]]
 ; CHECK-V6M: [[x]]:
 ; CHECK-V6M: .asciz "s\000\000"
 ; CHECK-V7: ldrb{{(.w)?}} r{{[0-9]*}}, [[x:.*]]
 ; CHECK-V7: [[x]]:
 ; CHECK-V7: .asciz "s\000\000"
 define void @test10(i8* %a) local_unnamed_addr #0 {