[MIR] Print target specific constant pools

[rovka]

Summary

This should enable us to test the generation of target-specific constant
pools, e.g. for ARM:

constants:

• id: 0 value: 'g(GOT_PREL)-(LPC0+8-.)' alignment: 4 isTargetSpecific: true

I intend to use this to test PIC support in GlobalISel for ARM.

I don't really know how to test it outside of that context, since the
existing MIR tests usually rely on parser support as well, and that
seems a bit trickier to add. I don't have an immediate use for that, but
I can put in the extra effort if people point me in the right direction
with it (i.e. where would we put the target-specific parsing?
Piggy-back it on the Subtarget, maybe?). Alternatively, I could try to
add a unit test, but the setup for that seems rather convoluted and I'm
not sure it's the best way to go about this.

Thoughts?

[arphaman]

It might not be worth doing full blown target-specific serialization for this, but it might be nice to add IsTargetSpecific optional field to the constant pool value YAML representation. This way the parser will at least know that it can't parse this kind of constant pool value when somebody tries to feed the printed MIR into llc.

The last time I thought about the target-specific serialization I had something like this in mind:
I think for target specific serialization it might make sense to inject a set of values into the YAML mapping. The parser would have to call into a target hook that could create a target-specific ConstantPoolValues from the YAML mapping (ARM has multiple subclasses, so you'd have to somehow store that information as well which might be an issue, although you could probably figure out the subclass just by looking at the keys in the YAML mapping). But again, I'm not sure it's worth it though, since ARM would have to provide YAML mappings for each individual enum that's used in constant pool values and the subclasses of ConstantPoolValue. Another problem is the MachineBasicBlock/Global value references, which are parsed after YAML is deserialized. ARM's parser might have to manually parse these (call into the parseMBBReference function`) which could be annoying. I think that could be partially solved using new types instead of raw strings the YAML traits. These types should expose only a function that can produce a real value (like MBB*), and not the raw string.

Another way to handle this would be to just print the value (as you did here) and provide a target-specific parsing hook and then the targets can just parse the printed value if they want to parse MIR with their target-specific values. But I think that's a bit crude and doesn't integrate with the rest of the MIR that well.

[rovka]

Thanks for the detailed response!

I added another field (isTargetSpecific) and made the parser produce a more meaningful error message when encountering one of those.

I'll look a bit more into the parser, but I'm not making any promises yet :)

[arphaman]

Sure thing, that can probably be done in another patch though.

The change LGTM. However, it lacks a test that verifies that the parser reports an error.

[rovka]

Refresh before commit. Now including a test for the error-path!

llvm/trunk/include/llvm/CodeGen/MIRYamlMapping.h

template <> struct MappingTraits<FixedMachineStackObject> {
 
   static const bool flow = true;
 };
 
 struct MachineConstantPoolValue {
   UnsignedValue ID;
   StringValue Value;
   unsigned Alignment = 0;
+  bool IsTargetSpecific = false;
   bool operator==(const MachineConstantPoolValue &Other) const {
     return ID == Other.ID && Value == Other.Value &&
-           Alignment == Other.Alignment;
+           Alignment == Other.Alignment &&
+           IsTargetSpecific == Other.IsTargetSpecific;
   }
 };
 
 template <> struct MappingTraits<MachineConstantPoolValue> {
   static void mapping(IO &YamlIO, MachineConstantPoolValue &Constant) {
     YamlIO.mapRequired("id", Constant.ID);
     YamlIO.mapOptional("value", Constant.Value, StringValue());
     YamlIO.mapOptional("alignment", Constant.Alignment, (unsigned)0);
+    YamlIO.mapOptional("isTargetSpecific", Constant.IsTargetSpecific, false);
   }
 };
 
 struct MachineJumpTable {
   struct Entry {
     UnsignedValue ID;
     std::vector<FlowStringValue> Blocks;
     bool operator==(const Entry &Other) const {

llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp

 
 bool MIRParserImpl::initializeConstantPool(PerFunctionMIParsingState &PFS,
     MachineConstantPool &ConstantPool, const yaml::MachineFunction &YamlMF) {
   DenseMap<unsigned, unsigned> &ConstantPoolSlots = PFS.ConstantPoolSlots;
   const MachineFunction &MF = PFS.MF;
   const auto &M = *MF.getFunction()->getParent();
   SMDiagnostic Error;
   for (const auto &YamlConstant : YamlMF.Constants) {
+    if (YamlConstant.IsTargetSpecific)
+      // FIXME: Support target-specific constant pools
+      return error(YamlConstant.Value.SourceRange.Start,
+                   "Can't parse target-specific constant pool entries yet");
     const Constant *Value = dyn_cast_or_null<Constant>(
         parseConstantValue(YamlConstant.Value.Value, Error, M));
     if (!Value)
       return error(Error, YamlConstant.Value.SourceRange);
     unsigned Alignment =
         YamlConstant.Alignment
             ? YamlConstant.Alignment
             : M.getDataLayout().getPrefTypeAlignment(Value->getType());

llvm/trunk/lib/CodeGen/MIRPrinter.cpp

for (const MachineFunction::VariableDbgInfo &DebugVar :
     }
   }
 }
 
 void MIRPrinter::convert(yaml::MachineFunction &MF,
                          const MachineConstantPool &ConstantPool) {
   unsigned ID = 0;
   for (const MachineConstantPoolEntry &Constant : ConstantPool.getConstants()) {
-    // TODO: Serialize target specific constant pool entries.
-    if (Constant.isMachineConstantPoolEntry())
-      llvm_unreachable("Can't print target specific constant pool entries yet");
-
-    yaml::MachineConstantPoolValue YamlConstant;
     std::string Str;
     raw_string_ostream StrOS(Str);
+    if (Constant.isMachineConstantPoolEntry()) {
+      Constant.Val.MachineCPVal->print(StrOS);
+    } else {
       Constant.Val.ConstVal->printAsOperand(StrOS);
+    }
+
+    yaml::MachineConstantPoolValue YamlConstant;
     YamlConstant.ID = ID++;
     YamlConstant.Value = StrOS.str();
     YamlConstant.Alignment = Constant.getAlignment();
+    YamlConstant.IsTargetSpecific = Constant.isMachineConstantPoolEntry();
+
     MF.Constants.push_back(YamlConstant);
   }
 }
 
 void MIRPrinter::convert(ModuleSlotTracker &MST,
                          yaml::MachineJumpTable &YamlJTI,
                          const MachineJumpTableInfo &JTI) {
   YamlJTI.Kind = JTI.getEntryKind();

llvm/trunk/test/CodeGen/MIR/ARM/target-constant-pools-error.mir

+# RUN: not llc -mtriple arm-unknown -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+--- |
+  target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
+  @g = private global i32 4
+  define void @target_constant_pool() { ret void }
+...
+---
+name:            target_constant_pool
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gpr, preferred-register: '' }
+  - { id: 1, class: gpr, preferred-register: '' }
+constants:
+  - id:              0
+  # CHECK: [[@LINE+1]]:22: Can't parse target-specific constant pool entries yet
+    value:           'g-(LPC0+8)'
+    alignment:       4
+    isTargetSpecific: true
+body:             |
+  bb.0.entry:
+    %0 = LDRi12 %const.0, 0, 14, _ :: (load 4 from constant-pool)
+    %1 = PICLDR killed %0, 0, 14, _ :: (dereferenceable load 4 from @g)
+    %r0 = COPY %1
+    BX_RET 14, _, implicit %r0
+
+...

llvm/trunk/test/CodeGen/MIR/X86/constant-pool.mir

--- |
   }
 ...
 ---
 # CHECK: name: test
 # CHECK: constants:
 # CHECK-NEXT: - id: 0
 # CHECK-NEXT:   value: 'double 3.250000e+00'
 # CHECK-NEXT:   alignment: 8
+# CHECK-NEXT:   isTargetSpecific: false
 # CHECK-NEXT: - id: 1
 # CHECK-NEXT:   value: 'float 6.250000e+00'
 # CHECK-NEXT:   alignment: 4
+# CHECK-NEXT:   isTargetSpecific: false
 name:            test
 constants:
   - id:          0
     value:       'double 3.250000e+00'
     alignment:   8
   - id:          1
     value:       'float 6.250000e+00'
     alignment:   4

 ...
 ---
 # Verify that alignment can be inferred:
 # CHECK: name: test2
 # CHECK: constants:
 # CHECK-NEXT: - id: 0
 # CHECK-NEXT:   value: 'double 3.250000e+00'
 # CHECK-NEXT:   alignment: 8
+# CHECK-NEXT:   isTargetSpecific: false
 # CHECK-NEXT: - id: 1
 # CHECK-NEXT:   value: 'float 6.250000e+00'
 # CHECK-NEXT:   alignment: 4
+# CHECK-NEXT:   isTargetSpecific: false
 name:            test2
 constants:
   - id:          0
     value:       'double 3.250000e+00'
   - id:          1
     value:       'float 6.250000e+00'
 body: |
   bb.0.entry:
     %xmm0 = ADDSDrm killed %xmm0, %rip, 1, _, %const.0, _
     %xmm1 = ADDSSrm killed %xmm1, %rip, 1, _, %const.1, _
     %xmm1 = CVTSS2SDrr killed %xmm1
     %xmm0 = MULSDrr killed %xmm0, killed %xmm1
     RETQ %xmm0
 ...
 ---
 # Verify that the non-standard alignments are respected:
 # CHECK: name: test3
 # CHECK: constants:
 # CHECK-NEXT: - id: 0
 # CHECK-NEXT:   value: 'double 3.250000e+00'
 # CHECK-NEXT:   alignment: 128
+# CHECK-NEXT:   isTargetSpecific: false
 # CHECK-NEXT: - id: 1
 # CHECK-NEXT:   value: 'float 6.250000e+00'
 # CHECK-NEXT:   alignment: 1
+# CHECK-NEXT:   isTargetSpecific: false
 name:            test3
 constants:
   - id:          0
     value:       'double 3.250000e+00'
     alignment:   128
   - id:          1
     value:       'float 6.250000e+00'
     alignment:   1