[ARM] Create SubtargetFeatures from build attributes

[samparker]

Summary

Currently, ELFObjectFile is used to build a SubtargetFeatures object for MIPS. I've moved the MIPS specific code into its own function and then also created an ARM specific function which builds the features from quering the build attributes. Together with D28281 it enables llvm-objdump to automatically create an accurate target.

[rengolin]

Also, please add negative tests, to make sure that we still refuse support when no attributes are found, or when those that are found restrict the target.

test/tools/llvm-objdump/ARM/v7a-subfeature.s

1

If you pass the -mattr, it doesn't need to read the .eabi_attributes, so what does this test really test?

[samparker]

test/tools/llvm-objdump/ARM/v7a-subfeature.s

1

As far as I can tell, the mattr is used by mc to detect valid instructions, but it is not used to encode the build attributes. So they're needed in the test so the attributes are encoded for objdump to read.

[rengolin]

test/tools/llvm-objdump/ARM/v7a-subfeature.s

1

That's my point. Here you have two ways to setup the attributes: -marttr and .eabi_attribute. What you need is to make sure that *both* ways work for your purposes.

If you create one test with -mattr and *another* with the same .eabi_attribute, making sure they're complete to describe what you need, than you covered both cases.

[samparker]

test/tools/llvm-objdump/ARM/v7a-subfeature.s

1

But this patch isn't about enabling llvm-mc to write attributes that are provided via the -mattr option. These tests surely only need to prove that they can use what is provided in the binary.

[rengolin]

test/tools/llvm-objdump/ARM/v7a-subfeature.s

1

Then I don't understand what the patch is about. ;)

What I see is an patch that reads the build attributes (not the -mattr) and adds those features to the target's list.

If you add the same attributes via -mattr, then reading the attributes will make no difference, ie. the test will pass without the build attributes being present.

To make sure the build attributes parsing and feature-setting behaviour works, you should *not* have those attributes already there (via -mattr) and let the build attributes be parsed, add the features and see if it works.

what am I missing here?

[samparker]

test/tools/llvm-objdump/ARM/v7a-subfeature.s

1

Ok. So the -mattr is required to enable llvm-mc to assemble the instructions because llvm-mc does not use the assembler directives to create a target for itself.

So if I remove +hwdiv-arm for an armv7a target, llvm-mc will fail to encode the instruction even if there is an assembler directive. I need to use -mattr to create a valid target for llvm-mc and I need the assembler directives so that llvm-mc encodes the build attributes into the its output.

[samparker]

Features can now also be disabled to override possible architecture defaults and I've also enabled +hwdiv for M and R class armv7 architectures. Added some negative tests too.

[rengolin]

Right, with the negative tests, it makes more sense now. Also, good split on the getFeatures. LGTM. Thanks!

[echristo]

llvm/trunk/include/llvm/Object/ObjectFile.h

270 ↗

(On Diff #84840)

Possibly should just be an internal helper routine and not a virtual alongside the normal API?

include/llvm/Object/ELFObjectFile.h

public:
   virtual elf_symbol_iterator_range getDynamicSymbolIterators() const = 0;
 
   elf_symbol_iterator_range symbols() const;
 
   static inline bool classof(const Binary *v) { return v->isELF(); }
 
   SubtargetFeatures getFeatures() const override;
 
+  SubtargetFeatures getMIPSFeatures() const override;
+
+  SubtargetFeatures getARMFeatures() const override;
+
   void setARMSubArch(Triple &TheTriple) const override;
 };
 
 class ELFSectionRef : public SectionRef {
 public:
   ELFSectionRef(const SectionRef &B) : SectionRef(B) {
     assert(isa<ELFObjectFileBase>(SectionRef::getObject()));
   }

include/llvm/Object/ObjectFile.h

 
   /// @brief The number of bytes used to represent an address in this object
   ///        file format.
   virtual uint8_t getBytesInAddress() const = 0;
 
   virtual StringRef getFileFormatName() const = 0;
   virtual /* Triple::ArchType */ unsigned getArch() const = 0;
   virtual SubtargetFeatures getFeatures() const = 0;
+  virtual SubtargetFeatures getMIPSFeatures() const { return SubtargetFeatures(); }
+  virtual SubtargetFeatures getARMFeatures() const { return SubtargetFeatures(); }
   virtual void setARMSubArch(Triple &TheTriple) const { }
 
   /// Returns platform-specific object flags, if any.
   virtual std::error_code getPlatformFlags(unsigned &Result) const {
     Result = 0;
     return object_error::invalid_file_type;
   }
 

include/llvm/Support/ARMAttributeParser.h

   void ParseSubsection(const uint8_t *Data, uint32_t Length);
 public:
   ARMAttributeParser(ScopedPrinter *SW) : SW(SW) {}
 
   ARMAttributeParser() : SW(nullptr) { }
 
   void Parse(ArrayRef<uint8_t> Section, bool isLittle);
 
+  bool hasAttribute(unsigned Tag) const {
+    return Attributes.count(Tag);
+  }
+
   unsigned getAttributeValue(unsigned Tag) const {
     return Attributes.find(Tag)->second.first;
   }
 };
 
 }
 
 #endif
 

lib/Object/ELFObjectFile.cpp

 //===- ELFObjectFile.cpp - ELF object file implementation -------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Part of the ELFObjectFile class implementation.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ARMAttributeParser.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace llvm {
 using namespace object;
 
 ELFObjectFileBase::ELFObjectFileBase(unsigned int Type, MemoryBufferRef Source)
     : ObjectFile(Type, Source) {}
 
if (Ident.first == ELF::ELFCLASS32) {
     return object_error::parse_failed;
   }
 
   if (EC)
     return EC;
   return std::move(R);
 }
 
-SubtargetFeatures ELFObjectFileBase::getFeatures() const {
-  switch (getEMachine()) {
-  case ELF::EM_MIPS: {
-    SubtargetFeatures Features;
-    unsigned PlatformFlags;
-    getPlatformFlags(PlatformFlags);
+SubtargetFeatures ELFObjectFileBase::getMIPSFeatures() const {
 
-    switch (PlatformFlags & ELF::EF_MIPS_ARCH) {
-    case ELF::EF_MIPS_ARCH_1:
-      break;
-    case ELF::EF_MIPS_ARCH_2:
-      Features.AddFeature("mips2");
-      break;
-    case ELF::EF_MIPS_ARCH_3:
-      Features.AddFeature("mips3");
-      break;
-    case ELF::EF_MIPS_ARCH_4:
-      Features.AddFeature("mips4");
-      break;
-    case ELF::EF_MIPS_ARCH_5:
-      Features.AddFeature("mips5");
-      break;
-    case ELF::EF_MIPS_ARCH_32:
-      Features.AddFeature("mips32");
-      break;
-    case ELF::EF_MIPS_ARCH_64:
-      Features.AddFeature("mips64");
-      break;
-    case ELF::EF_MIPS_ARCH_32R2:
-      Features.AddFeature("mips32r2");
-      break;
-    case ELF::EF_MIPS_ARCH_64R2:
-      Features.AddFeature("mips64r2");
-      break;
-    case ELF::EF_MIPS_ARCH_32R6:
-      Features.AddFeature("mips32r6");
-      break;
-    case ELF::EF_MIPS_ARCH_64R6:
-      Features.AddFeature("mips64r6");
-      break;
-    default:
-      llvm_unreachable("Unknown EF_MIPS_ARCH value");
-    }
 
-    switch (PlatformFlags & ELF::EF_MIPS_MACH) {
-    case ELF::EF_MIPS_MACH_NONE:
-      // No feature associated with this value.
-      break;
-    case ELF::EF_MIPS_MACH_OCTEON:
-      Features.AddFeature("cnmips");
-      break;
-    default:
-      llvm_unreachable("Unknown EF_MIPS_ARCH value");
-    }
 
-    if (PlatformFlags & ELF::EF_MIPS_ARCH_ASE_M16)
-      Features.AddFeature("mips16");
-    if (PlatformFlags & ELF::EF_MIPS_MICROMIPS)
-      Features.AddFeature("micromips");
 
-    return Features;
-  }
+
+SubtargetFeatures ELFObjectFileBase::getARMFeatures() const {
+  SubtargetFeatures Features;
+  ARMAttributeParser Attributes;
+  std::error_code EC = getBuildAttributes(Attributes);
+  if (EC)
+    return SubtargetFeatures();
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::CPU_arch_profile)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::CPU_arch_profile)) {
+    case ARMBuildAttrs::ApplicationProfile:
+      Features.AddFeature("aclass");
+      break;
+    case ARMBuildAttrs::RealTimeProfile:
+      Features.AddFeature("rclass");
+      break;
+    case ARMBuildAttrs::MicroControllerProfile:
+      Features.AddFeature("mclass");
+      break;
+    }
+  }
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::THUMB_ISA_use) &&
+      (Attributes.getAttributeValue(ARMBuildAttrs::THUMB_ISA_use) ==
+       ARMBuildAttrs::AllowThumb32))
+    Features.AddFeature("thumb2");
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::FP_arch)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::FP_arch)) {
+    case ARMBuildAttrs::AllowFPv2:
+      Features.AddFeature("vfp2");
+      break;
+    case ARMBuildAttrs::AllowFPv3A:
+    case ARMBuildAttrs::AllowFPv3B:
+      Features.AddFeature("vfp3");
+      break;
+    case ARMBuildAttrs::AllowFPv4A:
+    case ARMBuildAttrs::AllowFPv4B:
+      Features.AddFeature("vfp4");
+      break;
+    }
+  }
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::Advanced_SIMD_arch)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::Advanced_SIMD_arch)) {
+    default:
+      break;
+    case ARMBuildAttrs::AllowNeon:
+      Features.AddFeature("neon");
+      break;
+    case ARMBuildAttrs::AllowNeon2:
+      Features.AddFeature("neon");
+      Features.AddFeature("fp16");
+      break;
+    }
+  }
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::DIV_use) &&
+     (Attributes.getAttributeValue(ARMBuildAttrs::DIV_use) ==
+        ARMBuildAttrs::AllowDIVExt)) {
+    Features.AddFeature("hwdiv");
+    Features.AddFeature("hwdiv-arm");
+  }
+
+  return Features;
+}
+
+SubtargetFeatures ELFObjectFileBase::getFeatures() const {
+  switch (getEMachine()) {
+  case ELF::EM_MIPS:
+    return getMIPSFeatures();
+  case ELF::EM_ARM:
+    return getARMFeatures();
   default:
     return SubtargetFeatures();
   }
 }
 
 void ELFObjectFileBase::setARMSubArch(Triple &TheTriple) const {
   if (TheTriple.getSubArch() != Triple::NoSubArch)
     return;
 
   ARMAttributeParser Attributes;
   std::error_code EC = getBuildAttributes(Attributes);
+  if (EC)
+    return;
 
   std::string Triple;
   // Default to ARM, but use the triple if it's been set.
   if (TheTriple.getArch() == Triple::thumb ||
       TheTriple.getArch() == Triple::thumbeb)
     Triple = "thumb";
   else
     Triple = "arm";

test/tools/llvm-objdump/ARM/v6-subfeatures.s

+@ RUN: llvm-mc < %s -triple armv6 -mattr=+vfp2 -filetype=obj | llvm-objdump -triple=arm -d - | FileCheck %s
+
+.eabi_attribute Tag_FP_arch, 2 // VFP2
+
+vfp2:
+  vadd.f32 s0, s1, s2
+
+@CHECK-LABEL: vfp2
+@CHECK: 81 0a 30 ee vadd.f32 s0, s1, s2

test/tools/llvm-objdump/ARM/v7a-subfeature.s

+@ RUN: llvm-mc < %s -triple armv7a -mattr=+vfp3,+neon,+fp16,+hwdiv-arm -filetype=obj | llvm-objdump -triple=arm -d - | FileCheck %s
+
+.eabi_attribute Tag_FP_arch, 3 // VFP3
+.eabi_attribute Tag_Advanced_SIMD_arch, 2 // SIMDv1 with fp16
+.eabi_attribute Tag_DIV_use, 2 // permitted
+
+vfp2:
+  vmla.f32 s0, s1, s2
+
+@CHECK-LABEL: vfp2
+@CHECK: 81 0a 00 ee vmla.f32 s0, s1, s2
+
+vfp3:
+  vmov.f32 s0, #0.5
+
+@CHECK-LABEL: vfp3
+@CHECK: 00 0a b6 ee vmov.f32 s0, #5.000000e-01
+
+neon:
+  vmla.f32 d0, d1, d2
+
+@CHECK-LABEL: neon
+@CHECK: 12 0d 01 f2 vmla.f32 d0, d1, d2
+
+fp16:
+  vcvt.f32.f16 q0, d2
+
+@CHECK-LABEL: fp16
+@CHECK: 02 07 b6 f3  vcvt.f32.f16 q0, d2
+
+div:
+  udiv r0, r1, r2
+
+@CHECK-LABEL: div
+@CHECK: 11 f2 30 e7 udiv r0, r1, r2
+

test/tools/llvm-objdump/ARM/v7m-subfeatures.s

+@ RUN: llvm-mc < %s -triple armv7m -mattr=+vfp4 -filetype=obj | llvm-objdump -triple=thumb -d - | FileCheck %s
+
+.eabi_attribute Tag_FP_arch, 5 // VFP4
+
+.thumb
+vfp4:
+  vmov.f32 s0, #0.5
+
+@CHECK-LABEL: vfp4
+@CHECK: b6 ee 00 0a vmov.f32 s0, #5.000000e-01