[PATCH] D35152: Add some basic linker module symbols

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/COFF/Config.h:95
   llvm::SmallString<128> PDBPath;
+  std::vector<StringRef> Argv;
 
----------------
ruiu wrote:
> This can be ArrayRef.
In order to make it `ArrayRef` I have to make it `ArrayRef<const char *>`.  This is what I had in an earlier version of the patch, but I had problems on the call to `llvm::join()` (because you can't use `operator+` on two `const char*`.  So if I want to be able to use `StringRef`, it has to be `std::vector`


================
Comment at: lld/COFF/PDB.cpp:411
+
+  CS.Machine = CPUType::Intel80386;
+  CS.Flags = CompileSym3Flags::None;
----------------
smeenai wrote:
> zturner wrote:
> > ruiu wrote:
> > > Is this value the same on x86-64?
> > That's a good question, I should check.
> Probably dumb question: what about ARM (and AArch64 in the future)?
Whoever's interested in adding PDB support for `ARM and `AArch64` can update this code :)  The `CPUType` enumeration has a bunch of ARM values in the enumeration:

```
  ARM3 = 0x60,
  ARM4 = 0x61,
  ARM4T = 0x62,
  ARM5 = 0x63,
  ARM5T = 0x64,
  ARM6 = 0x65,
  ARM_XMAC = 0x66,
  ARM_WMMX = 0x67,
  ARM7 = 0x68,
  ARMNT = 0xf4,
```

the differences between which are not obvious to me :)  But the support is there (in theory) for whoever wants this.


https://reviews.llvm.org/D35152