[ARM] Add MemOp into LDRcp to enable removing

[yinma]

Summary

LDRcp should be deleted when the dest register is dead in register coalescing. Without MemOp, dead LDRcp will cause dead constant pool value which references to non-existing label.

[efriedma]

(It's customary to mention the target name in the title for patches which only touch one target, e.g. "[ARM] Add MemOp into LDRcp to enable removing", to help people quickly sort through llvm-commits.)

test/CodeGen/ARM/ldrcppic.ll

1 ↗

(On Diff #172813)

Probably worth adding a comment to explain this RUN line, since it's a bit unusual.

47 ↗

(On Diff #172813)

Please clean up the testcase to get rid of unnecessary attributes/branches/etc.

[efriedma]

test/CodeGen/ARM/ldrcppic.ll

1 ↗

(On Diff #172813)

I was hoping for a comment that would directly answer the question "why is this testcase using -filetype=obj, instead of emitting assembly"?

[efriedma]

LGTM

llvm/trunk/lib/Target/ARM/ARMFastISel.cpp

unsigned ARMFastISel::ARMLowerPICELF(const GlobalValue *GV,
   ARMConstantPoolValue *CPV = ARMConstantPoolConstant::Create(
       GV, ARMPCLabelIndex, ARMCP::CPValue, PCAdj,
       UseGOT_PREL ? ARMCP::GOT_PREL : ARMCP::no_modifier,
       /*AddCurrentAddress=*/UseGOT_PREL);
 
   unsigned ConstAlign =
       MF->getDataLayout().getPrefTypeAlignment(Type::getInt32PtrTy(*Context));
   unsigned Idx = MF->getConstantPool()->getConstantPoolIndex(CPV, ConstAlign);
+  MachineMemOperand *CPMMO =
+      MF->getMachineMemOperand(MachinePointerInfo::getConstantPool(*MF),
+                               MachineMemOperand::MOLoad, 4, 4);
 
   unsigned TempReg = MF->getRegInfo().createVirtualRegister(&ARM::rGPRRegClass);
   unsigned Opc = isThumb2 ? ARM::t2LDRpci : ARM::LDRcp;
   MachineInstrBuilder MIB =
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc), TempReg)
-          .addConstantPoolIndex(Idx);
+          .addConstantPoolIndex(Idx)
+          .addMemOperand(CPMMO);
   if (Opc == ARM::LDRcp)
     MIB.addImm(0);
   MIB.add(predOps(ARMCC::AL));
 
   // Fix the address by adding pc.
   unsigned DestReg = createResultReg(TLI.getRegClassFor(VT));
   Opc = Subtarget->isThumb() ? ARM::tPICADD : UseGOT_PREL ? ARM::PICLDR
                                                           : ARM::PICADD;
   DestReg = constrainOperandRegClass(TII.get(Opc), DestReg, 0);
   MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc), DestReg)
             .addReg(TempReg)
             .addImm(ARMPCLabelIndex);
+
   if (!Subtarget->isThumb())
     MIB.add(predOps(ARMCC::AL));
 
   if (UseGOT_PREL && Subtarget->isThumb()) {
     unsigned NewDestReg = createResultReg(TLI.getRegClassFor(VT));
     MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
                   TII.get(ARM::t2LDRi12), NewDestReg)
               .addReg(DestReg)

llvm/trunk/test/CodeGen/ARM/ldrcppic.ll

+; The failure is caused by ARM LDRcp/PICADD pairs. In PIC mode, the constant pool
+; need a label to do address computation. This label is emitted when backend emits
+; PICADD. When the target becomes dead, PICADD will be deleted. without this patch
+; LDRcp is dead but not being deleted. This will cause a dead contant pool entry
+; using a non existing label. This will cause an error in MC object emitting pass.
+
+; RUN: llc -relocation-model=pic -mcpu=cortex-a53 %s -filetype=obj -o - | llvm-nm - | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv8-unknown-linux-android"
+
+@_ZN15UsecaseSelector25AllowedImplDefinedFormatsE = external dso_local unnamed_addr constant <{ i32, i32, i32, i32, [12 x i32] }>, align 4
+
+; Function Attrs: noinline nounwind optnone sspstrong uwtable
+define dso_local fastcc void @_ZN15UsecaseSelector26IsAllowedImplDefinedFormatE15ChiBufferFormatj() unnamed_addr #1 align 2 {
+  br label %1
+
+; <label>:1:                                      ; preds = %13, %0
+  %2 = icmp ult i32 undef, 4
+  br i1 %2, label %3, label %14
+
+; <label>:3:                                      ; preds = %1
+  br i1 undef, label %4, label %13
+
+; <label>:4:                                      ; preds = %3
+  %5 = getelementptr inbounds [16 x i32], [16 x i32]* bitcast (<{ i32, i32, i32, i32, [12 x i32] }>* @_ZN15UsecaseSelector25AllowedImplDefinedFormatsE to [16 x i32]*), i32 0, i32 undef
+  %6 = load i32, i32* %5, align 4
+  %7 = icmp eq i32 10, %6
+  br i1 %7, label %9, label %8
+
+; <label>:8:                                      ; preds = %4
+  br i1 undef, label %9, label %12
+
+; <label>:9:                                      ; preds = %8, %4
+  br i1 undef, label %10, label %13
+
+; <label>:10:                                     ; preds = %9
+  br i1 undef, label %11, label %13
+
+; <label>:11:                                     ; preds = %10
+  br label %14
+
+; <label>:12:                                     ; preds = %8
+  br label %14
+
+; <label>:13:                                     ; preds = %10, %9, %3
+  br label %1
+
+; <label>:14:                                     ; preds = %12, %11, %1
+  ret void
+}
+
+attributes #1 = { noinline optnone }
+
+; CHECK: _ZN15UsecaseSelector26IsAllowedImplDefinedFormatE15ChiBufferFormatj
+