OpaquePtr: print byval types containing anonymous types correctly.

[t.p.northover]

Summary

While implementing inalloca types I discovered an issue with how we print attribute types (currently just byval). If the specified type references an anonymous Module-level one, the Attribute::getAsString function doesn't have the necessary context to figure out its number, so you'll see something like

%0 = type opaque

define void @foo(<{ %0* }>* byval(<{ %"type 0x7fe75580a800"* }>) %a) {
  ret void
}

This patch puts the Attribute printing logic into AsmWriter for this case, where TypePrinting is available to do the right thing. I only applied it to function argument attributes because that's the only attribute that can have a type currently, but would be happy to be more aggressive.

It's also worth noting that the Verifier is still affected by this issue when printing its messages, but I decided that was probably OK since there should be other context to figure out the problem location.

[t.p.northover]

Relised I should test all modified places that print attrs.

[arsenm]

LGTM

[t.p.northover]

Thanks Matt. Pushed:

To github.com:llvm/llvm-project.git

f26ed6e47cb8..e130eef58814  HEAD -> master

llvm/lib/IR/AsmWriter.cpp

void writeAtomic(const LLVMContext &Context,
                    SyncScope::ID SSID);
   void writeAtomicCmpXchg(const LLVMContext &Context,
                           AtomicOrdering SuccessOrdering,
                           AtomicOrdering FailureOrdering,
                           SyncScope::ID SSID);
 
   void writeAllMDNodes();
   void writeMDNode(unsigned Slot, const MDNode *Node);
+  void writeAttribute(const Attribute &Attr, bool InAttrGroup = false);
+  void writeAttributeSet(const AttributeSet &AttrSet, bool InAttrGroup = false);
   void writeAllAttributeGroups();
 
   void printTypeIdentities();
   void printGlobal(const GlobalVariable *GV);
   void printIndirectSymbol(const GlobalIndirectSymbol *GIS);
   void printComdat(const Comdat *C);
   void printFunction(const Function *F);
   void printArgument(const Argument *FA, AttributeSet Attrs);
void AssemblyWriter::writeParamOperand(const Value *Operand,
   if (!Operand) {
     Out << "<null operand!>";
     return;
   }
 
   // Print the type
   TypePrinter.print(Operand->getType(), Out);
   // Print parameter attributes list
-  if (Attrs.hasAttributes())
-    Out << ' ' << Attrs.getAsString();
+  if (Attrs.hasAttributes()) {
+    Out << ' ';
+    writeAttributeSet(Attrs);
+  }
   Out << ' ';
   // Print the operand
   WriteAsOperandInternal(Out, Operand, &TypePrinter, &Machine, TheModule);
 }
 
 void AssemblyWriter::writeOperandBundles(const CallBase *Call) {
   if (!Call->hasOperandBundles())
     return;
if (F->isDeclaration() && !IsForDebug) {
     for (unsigned I = 0, E = FT->getNumParams(); I != E; ++I) {
       // Insert commas as we go... the first arg doesn't get a comma
       if (I)
         Out << ", ";
       // Output type...
       TypePrinter.print(FT->getParamType(I), Out);
 
       AttributeSet ArgAttrs = Attrs.getParamAttributes(I);
-      if (ArgAttrs.hasAttributes())
-        Out << ' ' << ArgAttrs.getAsString();
+      if (ArgAttrs.hasAttributes()) {
+        Out << ' ';
+        writeAttributeSet(ArgAttrs);
+      }
     }
   } else {
     // The arguments are meaningful here, print them in detail.
     for (const Argument &Arg : F->args()) {
       // Insert commas as we go... the first arg doesn't get a comma
       if (Arg.getArgNo() != 0)
         Out << ", ";
       printArgument(&Arg, Attrs.getParamAttributes(Arg.getArgNo()));

 
 /// printArgument - This member is called for every argument that is passed into
 /// the function.  Simply print it out
 void AssemblyWriter::printArgument(const Argument *Arg, AttributeSet Attrs) {
   // Output type...
   TypePrinter.print(Arg->getType(), Out);
 
   // Output parameter attributes list
-  if (Attrs.hasAttributes())
-    Out << ' ' << Attrs.getAsString();
+  if (Attrs.hasAttributes()) {
+    Out << ' ';
+    writeAttributeSet(Attrs);
+  }
 
   // Output name, if available...
   if (Arg->hasName()) {
     Out << ' ';
     PrintLLVMName(Out, Arg);
   } else {
     int Slot = Machine.getLocalSlot(Arg);
     assert(Slot != -1 && "expect argument in function here");
for (unsigned i = 0, e = Nodes.size(); i != e; ++i) {
     writeMDNode(i, Nodes[i]);
   }
 }
 
 void AssemblyWriter::printMDNodeBody(const MDNode *Node) {
   WriteMDNodeBodyInternal(Out, Node, &TypePrinter, &Machine, TheModule);
 }
 
+void AssemblyWriter::writeAttribute(const Attribute &Attr, bool InAttrGroup) {
+  if (!Attr.isTypeAttribute()) {
+    Out << Attr.getAsString(InAttrGroup);
+    return;
+  }
+
+  assert(Attr.hasAttribute(Attribute::ByVal) && "unexpected type attr");
+
+  Out << "byval";
+  if (Type *Ty = Attr.getValueAsType()) {
+    Out << '(';
+    TypePrinter.print(Ty, Out);
+    Out << ')';
+  }
+}
+
+void AssemblyWriter::writeAttributeSet(const AttributeSet &AttrSet,
+                                       bool InAttrGroup) {
+  bool FirstAttr = true;
+  for (const auto &Attr : AttrSet) {
+    if (!FirstAttr)
+      Out << ' ';
+    writeAttribute(Attr, InAttrGroup);
+    FirstAttr = false;
+  }
+}
+
 void AssemblyWriter::writeAllAttributeGroups() {
   std::vector<std::pair<AttributeSet, unsigned>> asVec;
   asVec.resize(Machine.as_size());
 
   for (SlotTracker::as_iterator I = Machine.as_begin(), E = Machine.as_end();
        I != E; ++I)
     asVec[I->second] = *I;
 

llvm/test/Assembler/byval-type-attr.ll

fail:
   ret void
 }
 
 ; CHECK: declare void @baz([8 x i8]* byval([8 x i8]))
 %named_type = type [8 x i8]
 declare void @baz(%named_type* byval(%named_type))
 
 declare i32 @__gxx_personality_v0(...)
+
+%0 = type opaque
+
+; CHECK: define void @anon({ %0* }* byval({ %0* }) %arg)
+; CHECK:   call void @anon_callee({ %0* }* byval({ %0* }) %arg)
+define void @anon({ %0* }* byval({ %0* }) %arg) {
+  call void @anon_callee({ %0* }* byval({ %0* }) %arg)
+  ret void
+}
+
+; CHECK: declare void @anon_callee({ %0* }* byval({ %0* }))
+declare void @anon_callee({ %0* }* byval({ %0* }))