[llvm] r180750 - Revert the command line option patch. However, keep the part that makes this pass on Windows. I.e., we don't emit the target dependent attributes in a comment before the function.

[Reid Kleckner]

Sure, if you prefer it.  The TargetIndependent bool is a bit
confusing, but it also avoids some duplication.

diff --git a/lib/IR/AsmWriter.cpp b/lib/IR/AsmWriter.cpp
index dd0abba..7761127 100644
--- a/lib/IR/AsmWriter.cpp
+++ b/lib/IR/AsmWriter.cpp
@@ -1608,7 +1608,22 @@ void AssemblyWriter::printFunction(const Function *F) {
   const AttributeSet &Attrs = F->getAttributes();
   if (Attrs.hasAttributes(AttributeSet::FunctionIndex)) {
     AttributeSet AS = Attrs.getFnAttributes();
-    std::string AttrStr = AS.getAsString(AttributeSet::FunctionIndex, false);
+    std::string AttrStr;
+
+    unsigned Idx = 0;
+    for (unsigned E = AS.getNumSlots(); Idx != E; ++Idx)
+      if (AS.getSlotIndex(Idx) == AttributeSet::FunctionIndex)
+        break;

The core bug was that this loop failed to break.  Is it worth an
assert(Idx != E)?  We're already covered by the SmallVector bounds
check, but an explicit assert would express intent more clearly.

+    for (AttributeSet::iterator I = AS.begin(Idx), E = AS.end(Idx);
+         I != E; ++I) {
+      Attribute Attr = *I;
+      if (!Attr.isStringAttribute()) {
+        if (!AttrStr.empty()) AttrStr += ' ';
+        AttrStr += Attr.getAsString();
+      }
+    }
+

On Tue, Apr 30, 2013 at 10:00 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> On 30 April 2013 12:54, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> On 30 April 2013 08:36, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>> LGTM. Not passing a 64 number around when we don't need one is a good
>>> thing anyway.
>>
>> I have committed this as r180791.
>
> The attached patch reverts r180750  r180722. This should bring things
> back to where they were after Chris' revert in 180574. OK?
>
> Cheers,
> Rafael