[YAML] Add new method `yaml::IO::maskedBitSetCase()` to simplify handling of bitset flags with enumeration block

[atanasyan]

Summary

Some bit-set fields used in ELF file headers in fact contain two parts. The first one is a regular bit-field. The second one is an enumeraion. For example ELF header e_flags for MIPS target might contain the following values:

Bit-set values:

EF_MIPS_NOREORDER = 0x00000001
EF_MIPS_PIC       = 0x00000002
EF_MIPS_CPIC      = 0x00000004
EF_MIPS_ABI2      = 0x00000020

Enumeration:

EF_MIPS_ARCH_32   = 0x50000000
EF_MIPS_ARCH_64   = 0x60000000
EF_MIPS_ARCH_32R2 = 0x70000000
EF_MIPS_ARCH_64R2 = 0x80000000

For printing bit-sets we use the yaml::IO::bitSetCase(). It does not support bit-set/enumeration combinations and prints too many flags from an enumeration part. This patch fixes this problem. New method yaml::IO::maskedBitSetCase() handle "enumeration" part of bitset defined by provided mask.

[atanasyan]

Update the patch against the latest trunk state.

[atanasyan]

Ping?

[kledzik]

Please update the yaml documentation (llvm/docs/YamlIO.rst) with this new functionality too.

BTW, I had this same issue with mach-o in which the uint32_t section "flags" field has a 8-bit enum and 24-bits of flags. What I did was break them into two yaml fields.

include/llvm/Support/YAMLTraits.h

475–480

I think it would be cleaner to add a new method:

void maskedBitSetCase()

or

void bitSetCaseWithMask()

rather that adding that optional parameter, because now you have to check if the mask is zero (not used) and use different bit checking.

[atanasyan]

The updated patch has the following changes:

1. Keep the bitSetCase() unchanged and add new method maskedBitSetCase().
2. Update the yaml documentation.

I considered to break the Flags field into two parts. But I could not invent good names for these fields. The "enumeration" values have different meanings for different targets. Some targets do not use the Flags field at all. Moreover I am not sure that there is no target with more than one "enumeration" blocks in the Flag field.

[kledzik]

LGTM

[silvas]

docs/YamlIO.rst

418

typo: maskeBitSet

[atanasyan]

Thanks for review.

Closed by commit rL209504.

docs/YamlIO.rst

 is done when reading and any unknown string values will result in a error. With 
 the above schema, a same valid YAML document is:
 
 .. code-block:: yaml
 
     name:    Tom
     flags:   [ pointy, flat ]
 
+Sometimes a "flags" field might contains an enumeration part
+defined by a bit-mask.
+
+.. code-block:: c++
+
+    enum {
+      flagsFeatureA = 1,
+      flagsFeatureB = 2,
+      flagsFeatureC = 4,
+
+      flagsCPUMask = 24,
+
+      flagsCPU1 = 8,
+      flagsCPU2 = 16
+    };
+
+To support reading and writing such fields, you need to use the maskeBitSet()
+method and provide the bit values, their names and the enumeration mask.
+
+.. code-block:: c++
+
+    template <>
+    struct ScalarBitSetTraits<MyFlags> {
+      static void bitset(IO &io, MyFlags &value) {
+        io.bitSetCase(value, "featureA",  flagsFeatureA);
+        io.bitSetCase(value, "featureB",  flagsFeatureB);
+        io.bitSetCase(value, "featureC",  flagsFeatureC);
+        io.maskedBitSetCase(value, "CPU1",  flagsCPU1, flagsCPUMask);
+        io.maskedBitSetCase(value, "CPU2",  flagsCPU2, flagsCPUMask);
+      }
+    };
+
+YAML I/O (when writing) will apply the enumeration mask to the flags field,
+and compare the result and values from the bitset. As in case of a regular
+bitset, each that matches will cause the corresponding string to be added
+to the flow sequence.
 
 Custom Scalar
 -------------
 Sometimes for readability a scalar needs to be formatted in a custom way. For
 instance your internal data structure may use a integer for time (seconds since
 some epoch), but in YAML it would be much nicer to express that integer in 
 some time format (e.g. 4-May-2012 10:30pm).  YAML I/O has a way to support  
 custom formatting and parsing of scalar types by specializing ScalarTraits<> on

include/llvm/Support/YAMLTraits.h

public:
   // allow anonymous enum values to be used with LLVM_YAML_STRONG_TYPEDEF
   template <typename T>
   void enumCase(T &Val, const char* Str, const uint32_t ConstVal) {
     if ( matchEnumScalar(Str, outputting() && Val == static_cast<T>(ConstVal)) ) {
       Val = ConstVal;
     }
   }
 
   template <typename T>
   void bitSetCase(T &Val, const char* Str, const T ConstVal) {
     if ( bitSetMatch(Str, outputting() && (Val & ConstVal) == ConstVal) ) {
       Val = Val | ConstVal;
     }
   }
 
   // allow anonymous enum values to be used with LLVM_YAML_STRONG_TYPEDEF
   template <typename T>
   void bitSetCase(T &Val, const char* Str, const uint32_t ConstVal) {
     if ( bitSetMatch(Str, outputting() && (Val & ConstVal) == ConstVal) ) {
       Val = Val | ConstVal;
     }
   }
 
+  template <typename T>
+  void maskedBitSetCase(T &Val, const char *Str, T ConstVal, T Mask) {
+    if (bitSetMatch(Str, outputting() && (Val & Mask) == ConstVal))
+      Val = Val | ConstVal;
+  }
+
+  template <typename T>
+  void maskedBitSetCase(T &Val, const char *Str, uint32_t ConstVal,
+                        uint32_t Mask) {
+    if (bitSetMatch(Str, outputting() && (Val & Mask) == ConstVal))
+      Val = Val | ConstVal;
+  }
+
   void *getContext();
   void setContext(void *);
 
   template <typename T>
   void mapRequired(const char* Key, T& Val) {
     this->processKey(Key, Val, true);
   }
 

lib/Object/ELFYAML.cpp

 #undef ECase
 }
 
 void ScalarBitSetTraits<ELFYAML::ELF_EF>::bitset(IO &IO,
                                                  ELFYAML::ELF_EF &Value) {
   const auto *Object = static_cast<ELFYAML::Object *>(IO.getContext());
   assert(Object && "The IO context is not initialized");
 #define BCase(X) IO.bitSetCase(Value, #X, ELF::X);
+#define BCaseMask(X, M) IO.maskedBitSetCase(Value, #X, ELF::X, ELF::M);
   switch (Object->Header.Machine) {
   case ELF::EM_ARM:
     BCase(EF_ARM_SOFT_FLOAT)
     BCase(EF_ARM_VFP_FLOAT)
-    BCase(EF_ARM_EABI_UNKNOWN)
-    BCase(EF_ARM_EABI_VER1)
-    BCase(EF_ARM_EABI_VER2)
-    BCase(EF_ARM_EABI_VER3)
-    BCase(EF_ARM_EABI_VER4)
-    BCase(EF_ARM_EABI_VER5)
+    BCaseMask(EF_ARM_EABI_UNKNOWN, EF_ARM_EABIMASK)
+    BCaseMask(EF_ARM_EABI_VER1, EF_ARM_EABIMASK)
+    BCaseMask(EF_ARM_EABI_VER2, EF_ARM_EABIMASK)
+    BCaseMask(EF_ARM_EABI_VER3, EF_ARM_EABIMASK)
+    BCaseMask(EF_ARM_EABI_VER4, EF_ARM_EABIMASK)
+    BCaseMask(EF_ARM_EABI_VER5, EF_ARM_EABIMASK)
     break;
   case ELF::EM_MIPS:
     BCase(EF_MIPS_NOREORDER)
     BCase(EF_MIPS_PIC)
     BCase(EF_MIPS_CPIC)
     BCase(EF_MIPS_ABI2)
     BCase(EF_MIPS_32BITMODE)
     BCase(EF_MIPS_ABI_O32)
     BCase(EF_MIPS_MICROMIPS)
     BCase(EF_MIPS_ARCH_ASE_M16)
-    BCase(EF_MIPS_ARCH_1)
-    BCase(EF_MIPS_ARCH_2)
-    BCase(EF_MIPS_ARCH_3)
-    BCase(EF_MIPS_ARCH_4)
-    BCase(EF_MIPS_ARCH_5)
-    BCase(EF_MIPS_ARCH_32)
-    BCase(EF_MIPS_ARCH_64)
-    BCase(EF_MIPS_ARCH_32R2)
-    BCase(EF_MIPS_ARCH_64R2)
-    BCase(EF_MIPS_ARCH_32R6)
-    BCase(EF_MIPS_ARCH_64R6)
+    BCaseMask(EF_MIPS_ARCH_1, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_2, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_3, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_4, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_5, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_32, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_64, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_32R2, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_64R2, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_32R6, EF_MIPS_ARCH)
+    BCaseMask(EF_MIPS_ARCH_64R6, EF_MIPS_ARCH)
     break;
   case ELF::EM_HEXAGON:
     BCase(EF_HEXAGON_MACH_V2)
     BCase(EF_HEXAGON_MACH_V3)
     BCase(EF_HEXAGON_MACH_V4)
     BCase(EF_HEXAGON_MACH_V5)
     BCase(EF_HEXAGON_ISA_V2)
     BCase(EF_HEXAGON_ISA_V3)
     BCase(EF_HEXAGON_ISA_V4)
     BCase(EF_HEXAGON_ISA_V5)
     break;
   default:
     llvm_unreachable("Unsupported architecture");
   }
 #undef BCase
+#undef BCaseMask
 }
 
 void ScalarEnumerationTraits<ELFYAML::ELF_SHT>::enumeration(
     IO &IO, ELFYAML::ELF_SHT &Value) {
 #define ECase(X) IO.enumCase(Value, #X, ELF::X);
   ECase(SHT_NULL)
   ECase(SHT_PROGBITS)
   // No SHT_SYMTAB. Use the top-level `Symbols` key instead.

test/Object/obj2yaml.test

 COFF-X86-64-NEXT:     ComplexType: IMAGE_SYM_DTYPE_FUNCTION
 COFF-X86-64-NEXT:     StorageClass: IMAGE_SYM_CLASS_STATIC
 
 ELF-MIPSEL:      FileHeader:
 ELF-MIPSEL-NEXT:   Class:           ELFCLASS32
 ELF-MIPSEL-NEXT:   Data:            ELFDATA2LSB
 ELF-MIPSEL-NEXT:   Type:            ET_REL
 ELF-MIPSEL-NEXT:   Machine:         EM_MIPS
-ELF-MIPSEL-NEXT:   Flags:           [ EF_MIPS_NOREORDER, EF_MIPS_PIC, EF_MIPS_CPIC, EF_MIPS_ABI_O32, EF_MIPS_ARCH_1, EF_MIPS_ARCH_2, EF_MIPS_ARCH_5, EF_MIPS_ARCH_32 ]
+ELF-MIPSEL-NEXT:   Flags:           [ EF_MIPS_NOREORDER, EF_MIPS_PIC, EF_MIPS_CPIC, EF_MIPS_ABI_O32, EF_MIPS_ARCH_32 ]
 ELF-MIPSEL-NEXT: Sections:
 ELF-MIPSEL-NEXT:   - Name:            .text
 ELF-MIPSEL-NEXT:     Type:            SHT_PROGBITS
 ELF-MIPSEL-NEXT:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
 ELF-MIPSEL-NEXT:     AddressAlign:    0x0000000000000004
 ELF-MIPSEL-NEXT:     Content:         0000023C00004224E8FFBD271400BFAF1000B0AF218059000000018E000024240000198E09F8200321E000020000198E09F8200321E00002000002241000B08F1400BF8F0800E0031800BD27
 ELF-MIPSEL-NEXT:   - Name:            .rel.text
 ELF-MIPSEL-NEXT:     Type:            SHT_REL

 ELF-MIPSEL-NEXT:     - Name:            _gp_disp
 ELF-MIPSEL-NEXT:     - Name:            puts
 
 ELF-MIPS64EL:      FileHeader:
 ELF-MIPS64EL-NEXT:   Class:           ELFCLASS64
 ELF-MIPS64EL-NEXT:   Data:            ELFDATA2LSB
 ELF-MIPS64EL-NEXT:   Type:            ET_REL
 ELF-MIPS64EL-NEXT:   Machine:         EM_MIPS
-ELF-MIPS64EL-NEXT:   Flags:           [ EF_MIPS_ARCH_1, EF_MIPS_ARCH_3 ]
+ELF-MIPS64EL-NEXT:   Flags:           [ EF_MIPS_ARCH_3 ]
 ELF-MIPS64EL-NEXT: Sections:
 ELF-MIPS64EL-NEXT:   - Name:            .text
 ELF-MIPS64EL-NEXT:     Type:            SHT_PROGBITS
 ELF-MIPS64EL-NEXT:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
 ELF-MIPS64EL-NEXT:     AddressAlign:    0x0000000000000010
 ELF-MIPS64EL-NEXT:     Content:         ''
 ELF-MIPS64EL-NEXT:   - Name:            .data
 ELF-MIPS64EL-NEXT:     Type:            SHT_PROGBITS