[ARM] Use TargetMachine over Subtarget in ARMAsmPrinter

[davemgreen]

The subtarget may not be set if no functions are present in the module. Attempt to use the TargetMachine directly in more cases.

Fixes #165422
Fixes #167577

[llvmbot]

@llvm/pr-subscribers-backend-arm

Author: David Green (davemgreen)

Changes

The subtarget may not be set if no functions are present in the module. Attempt to use the TargetMachine directly in more cases.

Fixes #165422

---

Full diff: https://github.com/llvm/llvm-project/pull/166329.diff

4 Files Affected:

• (modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+12-11)
• (modified) llvm/lib/Target/ARM/ARMSubtarget.cpp (+7-2)
• (modified) llvm/lib/Target/ARM/ARMSubtarget.h (+2)
• (added) llvm/test/CodeGen/ARM/xxstructor-nodef.ll (+7)

diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 36b99087e0a32..60f5e9828c284 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -97,7 +97,8 @@ void ARMAsmPrinter::emitXXStructor(const DataLayout &DL, const Constant *CV) {
 
   const MCExpr *E = MCSymbolRefExpr::create(
       GetARMGVSymbol(GV, ARMII::MO_NO_FLAG),
-      (Subtarget->isTargetELF() ? ARM::S_TARGET1 : ARM::S_None), OutContext);
+      (TM.getTargetTriple().isOSBinFormatELF() ? ARM::S_TARGET1 : ARM::S_None),
+      OutContext);
 
   OutStreamer->emitValue(E, Size);
 }
@@ -595,8 +596,7 @@ void ARMAsmPrinter::emitEndOfAsmFile(Module &M) {
   ARMTargetStreamer &ATS = static_cast<ARMTargetStreamer &>(TS);
 
   if (OptimizationGoals > 0 &&
-      (Subtarget->isTargetAEABI() || Subtarget->isTargetGNUAEABI() ||
-       Subtarget->isTargetMuslAEABI()))
+      (TT.isTargetAEABI() || TT.isTargetGNUAEABI() || TT.isTargetMuslAEABI()))
     ATS.emitAttribute(ARMBuildAttrs::ABI_optimization_goals, OptimizationGoals);
   OptimizationGoals = -1;
 
@@ -884,9 +884,10 @@ static uint8_t getModifierSpecifier(ARMCP::ARMCPModifier Modifier) {
 
 MCSymbol *ARMAsmPrinter::GetARMGVSymbol(const GlobalValue *GV,
                                         unsigned char TargetFlags) {
-  if (Subtarget->isTargetMachO()) {
-    bool IsIndirect =
-        (TargetFlags & ARMII::MO_NONLAZY) && Subtarget->isGVIndirectSymbol(GV);
+  const Triple &TT = TM.getTargetTriple();
+  if (TT.isOSBinFormatMachO()) {
+    bool IsIndirect = (TargetFlags & ARMII::MO_NONLAZY) &&
+                      ARMSubtarget::isGVIndirectSymbol(TM, GV);
 
     if (!IsIndirect)
       return getSymbol(GV);
@@ -903,9 +904,8 @@ MCSymbol *ARMAsmPrinter::GetARMGVSymbol(const GlobalValue *GV,
       StubSym = MachineModuleInfoImpl::StubValueTy(getSymbol(GV),
                                                    !GV->hasInternalLinkage());
     return MCSym;
-  } else if (Subtarget->isTargetCOFF()) {
-    assert(Subtarget->isTargetWindows() &&
-           "Windows is the only supported COFF target");
+  } else if (TT.isOSBinFormatCOFF()) {
+    assert(TT.isOSWindows() && "Windows is the only supported COFF target");
 
     bool IsIndirect =
         (TargetFlags & (ARMII::MO_DLLIMPORT | ARMII::MO_COFFSTUB));
@@ -932,7 +932,7 @@ MCSymbol *ARMAsmPrinter::GetARMGVSymbol(const GlobalValue *GV,
     }
 
     return MCSym;
-  } else if (Subtarget->isTargetELF()) {
+  } else if (TT.isOSBinFormatELF()) {
     return getSymbolPreferLocal(*GV);
   }
   llvm_unreachable("unexpected target");
@@ -978,7 +978,8 @@ void ARMAsmPrinter::emitMachineConstantPoolValue(
 
     // On Darwin, const-pool entries may get the "FOO$non_lazy_ptr" mangling, so
     // flag the global as MO_NONLAZY.
-    unsigned char TF = Subtarget->isTargetMachO() ? ARMII::MO_NONLAZY : 0;
+    unsigned char TF =
+        TM.getTargetTriple().isOSBinFormatMachO() ? ARMII::MO_NONLAZY : 0;
     MCSym = GetARMGVSymbol(GV, TF);
   } else if (ACPV->isMachineBasicBlock()) {
     const MachineBasicBlock *MBB = cast<ARMConstantPoolMBB>(ACPV)->getMBB();
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 58bc338b25856..defffd6c9340e 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -317,20 +317,25 @@ bool ARMSubtarget::isRWPI() const {
          TM.getRelocationModel() == Reloc::ROPI_RWPI;
 }
 
-bool ARMSubtarget::isGVIndirectSymbol(const GlobalValue *GV) const {
+bool ARMSubtarget::isGVIndirectSymbol(const TargetMachine &TM,
+                                      const GlobalValue *GV) {
   if (!TM.shouldAssumeDSOLocal(GV))
     return true;
 
   // 32 bit macho has no relocation for a-b if a is undefined, even if b is in
   // the section that is being relocated. This means we have to use o load even
   // for GVs that are known to be local to the dso.
-  if (isTargetMachO() && TM.isPositionIndependent() &&
+  if (TM.getTargetTriple().isOSBinFormatMachO() && TM.isPositionIndependent() &&
       (GV->isDeclarationForLinker() || GV->hasCommonLinkage()))
     return true;
 
   return false;
 }
 
+bool ARMSubtarget::isGVIndirectSymbol(const GlobalValue *GV) const {
+  return isGVIndirectSymbol(TM, GV);
+}
+
 bool ARMSubtarget::isGVInGOT(const GlobalValue *GV) const {
   return isTargetELF() && TM.isPositionIndependent() && !GV->isDSOLocal();
 }
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.h b/llvm/lib/Target/ARM/ARMSubtarget.h
index 4a0883cc662e7..95fb62da8cf01 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.h
+++ b/llvm/lib/Target/ARM/ARMSubtarget.h
@@ -464,6 +464,8 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
   }
 
   /// True if the GV will be accessed via an indirect symbol.
+  static bool isGVIndirectSymbol(const TargetMachine &TM,
+                                 const GlobalValue *GV);
   bool isGVIndirectSymbol(const GlobalValue *GV) const;
 
   /// Returns the constant pool modifier needed to access the GV.
diff --git a/llvm/test/CodeGen/ARM/xxstructor-nodef.ll b/llvm/test/CodeGen/ARM/xxstructor-nodef.ll
new file mode 100644
index 0000000000000..db17b2b1c21ab
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/xxstructor-nodef.ll
@@ -0,0 +1,7 @@
+; RUN: llc -mtriple=arm-unknown-linux-gnueabihf < %s | FileCheck %s
+
+; This test contains a llvm.global_ctors with no other definitions. Make sure we do not crash in that case.
+; CHECK: .section        .init_array,"aw",%init_array
+
+declare  ccc void @ghczmbignum_GHCziNumziBackendziSelected_init__prof_init()
+@llvm.global_ctors = appending global [1 x {i32, void ()*, i8* }] [{i32, void ()*, i8* }{i32  65535, void ()*  @ghczmbignum_GHCziNumziBackendziSelected_init__prof_init, i8*  null } ]

[smithp35]

This looks good to me. All the changes relate to things that don't vary per function like the object file format.

Will be good to leave some time for reviewers in the US time zone to comment.

[efriedma-quic]

Can we completely delete the "Subtarget" variable from ArmAsmPrinter? It seems easy to misuse, and I think everywhere that legitimately uses a Subtarget should be able to get it from the corresponding MachineFunction.

[efriedma-quic]

Please move this implementation into ARMTargetMachine.

[davemgreen]

Can we completely delete the "Subtarget" variable from ArmAsmPrinter? It seems easy to misuse, and I think everywhere that legitimately uses a Subtarget should be able to get it from the corresponding MachineFunction.

Yeah I think that might be a good idea. We have had another report of this so it would be good to get it fixed, I'll see about doing it in a follow-up.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[efriedma-quic]

LGTM