[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

2

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

2

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

2

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

2

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

2

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

2

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

 
   SubtargetFeatures getFeatures() const override;
 
+  SubtargetFeatures getMIPSFeatures() const override;
+
+  SubtargetFeatures getARMFeatures() const override;
+
   void setARMSubArch(Triple &TheTriple) const override;
 };
 

include/llvm/Object/ObjectFile.h

   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.

include/llvm/Support/ARMAttributeParser.h

 
   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;
   }

lib/Object/ELFObjectFile.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ARMAttributeParser.h"
 #include "llvm/Support/MathExtras.h"
 
 namespace llvm {

   return std::move(R);
 }
 
-SubtargetFeatures ELFObjectFileBase::getFeatures() const {
-  switch (getEMachine()) {
-  case ELF::EM_MIPS: {
-    SubtargetFeatures Features;
-    unsigned PlatformFlags;
-    getPlatformFlags(PlatformFlags);
+SubtargetFeatures ELFObjectFileBase::getMIPSFeatures() const {
+  SubtargetFeatures Features;
+  unsigned PlatformFlags;
+  getPlatformFlags(PlatformFlags);
+
+  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();
 
-    switch (PlatformFlags & ELF::EF_MIPS_ARCH) {
-    case ELF::EF_MIPS_ARCH_1:
+
+  // both ARMv7-M and R have to support thumb hardware div
+  bool isV7 = false;
+  if (Attributes.hasAttribute(ARMBuildAttrs::CPU_arch))
+    isV7 = Attributes.getAttributeValue(ARMBuildAttrs::CPU_arch)
+      == ARMBuildAttrs::v7;
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::CPU_arch_profile)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::CPU_arch_profile)) {
+    case ARMBuildAttrs::ApplicationProfile:
+      Features.AddFeature("aclass");
       break;
-    case ELF::EF_MIPS_ARCH_2:
-      Features.AddFeature("mips2");
+    case ARMBuildAttrs::RealTimeProfile:
+      Features.AddFeature("rclass");
+      if (isV7)
+        Features.AddFeature("hwdiv");
       break;
-    case ELF::EF_MIPS_ARCH_3:
-      Features.AddFeature("mips3");
+    case ARMBuildAttrs::MicroControllerProfile:
+      Features.AddFeature("mclass");
+      if (isV7)
+        Features.AddFeature("hwdiv");
       break;
-    case ELF::EF_MIPS_ARCH_4:
-      Features.AddFeature("mips4");
+    }
+  }
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::THUMB_ISA_use)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::THUMB_ISA_use)) {
+    default:
       break;
-    case ELF::EF_MIPS_ARCH_5:
-      Features.AddFeature("mips5");
+    case ARMBuildAttrs::Not_Allowed:
+      Features.AddFeature("thumb", false);
+      Features.AddFeature("thumb2", false);
       break;
-    case ELF::EF_MIPS_ARCH_32:
-      Features.AddFeature("mips32");
+    case ARMBuildAttrs::AllowThumb32:
+      Features.AddFeature("thumb2");
       break;
-    case ELF::EF_MIPS_ARCH_64:
-      Features.AddFeature("mips64");
+    }
+  }
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::FP_arch)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::FP_arch)) {
+    default:
       break;
-    case ELF::EF_MIPS_ARCH_32R2:
-      Features.AddFeature("mips32r2");
+    case ARMBuildAttrs::Not_Allowed:
+      Features.AddFeature("vfp2", false);
+      Features.AddFeature("vfp3", false);
+      Features.AddFeature("vfp4", false);
       break;
-    case ELF::EF_MIPS_ARCH_64R2:
-      Features.AddFeature("mips64r2");
+    case ARMBuildAttrs::AllowFPv2:
+      Features.AddFeature("vfp2");
       break;
-    case ELF::EF_MIPS_ARCH_32R6:
-      Features.AddFeature("mips32r6");
+    case ARMBuildAttrs::AllowFPv3A:
+    case ARMBuildAttrs::AllowFPv3B:
+      Features.AddFeature("vfp3");
       break;
-    case ELF::EF_MIPS_ARCH_64R6:
-      Features.AddFeature("mips64r6");
+    case ARMBuildAttrs::AllowFPv4A:
+    case ARMBuildAttrs::AllowFPv4B:
+      Features.AddFeature("vfp4");
       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.
+  if (Attributes.hasAttribute(ARMBuildAttrs::Advanced_SIMD_arch)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::Advanced_SIMD_arch)) {
+    default:
+      break;
+    case ARMBuildAttrs::Not_Allowed:
+      Features.AddFeature("neon", false);
+      Features.AddFeature("fp16", false);
       break;
-    case ELF::EF_MIPS_MACH_OCTEON:
-      Features.AddFeature("cnmips");
+    case ARMBuildAttrs::AllowNeon:
+      Features.AddFeature("neon");
       break;
+    case ARMBuildAttrs::AllowNeon2:
+      Features.AddFeature("neon");
+      Features.AddFeature("fp16");
+      break;
+    }
+  }
+
+  if (Attributes.hasAttribute(ARMBuildAttrs::DIV_use)) {
+    switch(Attributes.getAttributeValue(ARMBuildAttrs::DIV_use)) {
     default:
-      llvm_unreachable("Unknown EF_MIPS_ARCH value");
+      break;
+    case ARMBuildAttrs::DisallowDIV:
+      Features.AddFeature("hwdiv", false);
+      Features.AddFeature("hwdiv-arm", false);
+      break;
+    case ARMBuildAttrs::AllowDIVExt:
+      Features.AddFeature("hwdiv");
+      Features.AddFeature("hwdiv-arm");
+      break;
     }
+  }
 
-    if (PlatformFlags & ELF::EF_MIPS_ARCH_ASE_M16)
-      Features.AddFeature("mips16");
-    if (PlatformFlags & ELF::EF_MIPS_MICROMIPS)
-      Features.AddFeature("micromips");
+  return Features;
+}
 
-    return Features;
-  }
+SubtargetFeatures ELFObjectFileBase::getFeatures() const {
+  switch (getEMachine()) {
+  case ELF::EM_MIPS:
+    return getMIPSFeatures();
+  case ELF::EM_ARM:
+    return getARMFeatures();
   default:
     return SubtargetFeatures();
   }

 
   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.

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

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

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-neg-subfeature.s

+@ RUN: llvm-mc < %s -triple armv7a -mattr=+vfp3,+neon,+fp16,+hwdiv-arm,+hwdiv -filetype=obj | llvm-objdump -triple=arm -d - | FileCheck %s
+@ RUN: llvm-mc < %s -triple armv7a -mattr=+vfp3,+neon,+fp16,+hwdiv-arm,+hwdiv -filetype=obj | llvm-objdump -triple=thumb -d - | FileCheck %s --check-prefix=CHECK-THUMB
+
+.eabi_attribute Tag_FP_arch, 0 // disallow vfp
+
+vfp2:
+  vmla.f32 s0, s1, s2
+
+@CHECK-LABEL: vfp2
+@CHECK-NOT: 81 0a 00 ee vmla.f32 s0, s1, s2
+@CHECK: unknown
+
+vfp3:
+  vmov.f32 s0, #0.5
+
+@CHECK-LABEL: vfp3
+@CHECK-NOT: 00 0a b6 ee vmov.f32 s0, #5.000000e-01
+
+neon:
+  vmla.f32 d0, d1, d2
+
+@CHECK-LABEL: neon
+@CHECK-NOT: 12 0d 01 f2 vmla.f32 d0, d1, d2
+@CHECK: unknown
+
+fp16:
+  vcvt.f32.f16 q0, d2
+
+@CHECK-LABEL: fp16
+@CHECK-NOT: 02 07 b6 f3  vcvt.f32.f16 q0, d2
+
+div_arm:
+  udiv r0, r1, r2
+
+@CHECK-LABEL: div_arm
+@CHECK-NOT: 11 f2 30 e7 udiv r0, r1, r2
+@CHECK: unknown
+
+.thumb
+div_thumb:
+  udiv r0, r1, r2
+
+@CHECK-LABEL: div_thumb
+@CHECK-THUMB-NOT: b1 fb f2 f0 udiv r0, r1, r2

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-neg-subfeatures.s

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

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_CPU_arch, 10 // v7
+.eabi_attribute Tag_CPU_arch_profile, 0x4D // 'M' profile
+.eabi_attribute Tag_FP_arch, 5 // VFP4
+
+.thumb
+vfp2:
+  vmla.f32 s0, s1, s2
+
+@CHECK-LABEL: vfp2
+@CHECK: 00 ee 81 0a vmla.f32 s0, s1, s2
+
+.thumb
+vfp4:
+  vmov.f32 s0, #0.5
+
+@CHECK-LABEL: vfp4
+@CHECK: b6 ee 00 0a vmov.f32 s0, #5.000000e-01
+
+.thumb
+div:
+  udiv r0, r1, r2
+
+@CHECK-LABEL: div
+@CHECK: b1 fb f2 f0 udiv r0, r1, r2

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

+@ RUN: llvm-mc < %s -triple armv7r -mattr=+hwdiv-arm -filetype=obj | llvm-objdump -triple=thumb -d - | FileCheck %s
+@ RUN: llvm-mc < %s -triple armv7r -mattr=+hwdiv-arm -filetype=obj | llvm-objdump -triple=arm -d - | FileCheck %s --check-prefix=CHECK-ARM
+
+.eabi_attribute Tag_CPU_arch, 10 // v7
+.eabi_attribute Tag_CPU_arch_profile, 0x52 // 'R' profile
+
+.arm
+div_arm:
+  udiv r0, r1, r2
+
+@CHECK-LABEL: div_arm
+@CHECK-NOT: udiv r0, r1, r2
+@CHECK-ARM-NOT: udiv r0, r1, r2
+
+.thumb
+div_thumb:
+  udiv r0, r1, r2
+
+@CHECK-LABEL: div_thumb
+@CHECK: b1 fb f2 f0 udiv r0, r1, r2