[PATCH] #pragma vectorize

[Aaron Ballman]

On Tue, May 13, 2014 at 9:56 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Mon, May 12, 2014 at 7:20 PM, Tyler Nowicki <tnowicki at apple.com> wrote:

<snip>

>> Index: lib/AST/StmtPrinter.cpp
>> ===================================================================
>> --- lib/AST/StmtPrinter.cpp (revision 208638)
>> +++ lib/AST/StmtPrinter.cpp (working copy)
>> @@ -168,19 +168,32 @@
>>  }
>>
>>  void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
>> -  OS << "[[";
>>    bool first = true;
>> -  for (ArrayRef<const Attr*>::iterator it = Node->getAttrs().begin(),
>> -                                       end = Node->getAttrs().end();
>> -                                       it != end; ++it) {
>> -    if (!first) {
>> -      OS << ", ";
>> -      first = false;
>> +  std::string raw_attr_os;
>> +  llvm::raw_string_ostream AttrOS(raw_attr_os);
>> +  for (auto I = Node->getAttrs().rbegin(), E = Node->getAttrs().rend();
>> +       I != E; ++I) {
>> +    // FIXME: This hack will be removed when printPretty
>> +    // has been modified to print pretty pragmas.
>> +    if (dyn_cast<LoopHintAttr>(*I)) {
>> +      const LoopHintAttr *LHA = cast<LoopHintAttr>(*I);
>> +      OS << "#pragma loop ";
>> +      LHA->print(OS, Policy);
>> +    } else {
>> +      if (!first) {
>> +        AttrOS << ", ";
>> +        first = false;
>> +      }
>> +      // TODO: check this
>> +      (*I)->printPretty(AttrOS, Policy);
>>      }
>> -    // TODO: check this
>> -    (*it)->printPretty(OS, Policy);
>>    }
>> -  OS << "]] ";
>> +
>> +  // Check to see if any attributes were printed.
>> +  StringRef AttrStr = AttrOS.str();
>> +  if (!AttrStr.empty())
>> +    OS << "[[" << AttrStr << "]] ";
>> +
>>    PrintStmt(Node->getSubStmt(), 0);
>>  }

Just to warn you, I just modified this code in r208706 because it was
entirely wrong. It was printing the attribute introducer twice (once
manually, and once from printPretty). What's more, it was printing an
extraneous comma, which would then be invalid syntax. So you are
likely to hit some merge conflicts from that.

When you reintroduce your changes, I think I would prefer to leave the
iteration in its current (forward) order instead of reverse order (as
you have it in your patch). That's simply a bug, and working around it
here is likely to ensure that bug sticks around even longer. Once you
have introduced your changes, I'd appreciate an extra test case which
uses -ast-print and demonstrates the reversed order (I am okay with us
XFAILing that test) so that gives us a target test to use to verify
the fix when it does happen.

Thanks, and sorry for the merge conflict!

~Aaron