[PATCH] CodeGen: Stick constant pool entries in COMDAT sections for WinCOFF

[David Majnemer]

================
Comment at: include/llvm/ADT/StringExtras.h:56
@@ -55,3 +55,3 @@
 template<typename IntTy>
-static inline char *utohex_buffer(IntTy X, char *BufferEnd) {
+static inline char *utohex_buffer(IntTy X, char *BufferEnd, bool LowerCase = false) {
   char *BufPtr = BufferEnd;
----------------
Jim Grosbach wrote:
> These changes are distinct and should be a separate patch.
> 
> Also, documentation comment needs updated to match.
Done.

================
Comment at: include/llvm/CodeGen/MachineConstantPool.h:124
@@ -122,1 +123,3 @@
+
+  SectionKind getSectionKind(const TargetMachine &TM) const;
 };
----------------
Jim Grosbach wrote:
> Moving this to a helper function makes sense, but should be a separate patch.
> 
> It should take a datalayout argument, too, not the target machine.
Done.

================
Comment at: include/llvm/CodeGen/TargetLoweringObjectFileImpl.h:47
@@ -48,1 +46,3 @@
+  const MCSection *getSectionForConstant(SectionKind Kind, const Constant *C,
+                                         MCStreamer &Streamer) const override;
 
----------------
Jim Grosbach wrote:
> Having these take an MCStreamer reference is really awkward. If that's necessary, it implies something isn't right about the layering elsewhere.
Fixed.

================
Comment at: lib/Target/X86/X86TargetObjectFile.cpp:166
@@ +165,3 @@
+        if (Sym->isUndefined())
+          Streamer.EmitSymbolAttribute(Sym, MCSA_Global);
+        return S;
----------------
Jim Grosbach wrote:
> OK, here's the layering issue. These functions should be strictly lookup, but here we're mutating things by calling an output function on the streamer. This function isn't the right place for figuring out the symbol names and emitting them to the streamer. GetCPISymbol() is a better place for that.
Fixed.

http://reviews.llvm.org/D4482