[llvm][AsmPrinter] Restore source location to register clobber warning

[DavidSpickett]

Summary

Since 5de2d189e6ad466a1f0616195e8c524a4eb3cbc0 this particular warning
hasn't had the location of the source file containing the inline
assembly.

Fix this by reporting via LLVMContext. Which means that we no longer
have the "instantiated into assembly here" lines but they were going to
point to the start of the inline asm string anyway.

This message is already tested via IR in llvm. However we won't have
the required location info there so I've added a C file test in clang
to cover it.
(though strictly, this is testing llvm code)

[DavidSpickett]

This works as expected, let me know if this is the intended use relative to the changes you made.

The existing test in llvm llvm/test/CodeGen/AArch64/inline-asm-clobber.ll never reported the location in the .ll file so I can't test it there. (which might be its own issue) I looked for any similar tests but didn't find anything, testing from clang with a C file seems like the only way.

clang/test/Misc/inline-asm-clobber-warning.c

27

I think this ^ is wrong because we don't/can't account for the asm printer tabbing in the instruction.

I still wanted to check that there are ^ just because it's one aspect of the location information.

[ychen]

clang/test/Misc/inline-asm-clobber-warning.c

27

This probably is due to Loc is not right.

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

540–541

For this diagnose, it is better to use LLVMContext since the location inside inlineasm does not matter. There is an example above.

Msg << "invalid operand in inline asm: '" << AsmStr << "'";
MMI->getModule()->getContext().emitError(LocCookie, Msg.str());

[DavidSpickett]

• Report via LLVMContext

[ychen]

Looks good except for one suggestion and the patch need rebasing on an upstream commit.

llvm/include/llvm/IR/LLVMContext.h

300

It would be best to avoid adding these two methods for single-use since LLVMContext.h is included everywhere. We could add them once it turns out to be needed in other places. Could we use LLVMContext::diagnose instead?

[DavidSpickett]

• Use diagnose instead of adding methods to LLVMContext
• clang-format the test file

[ychen]

LGTM. Thanks.

clang/test/Misc/inline-asm-clobber-warning.c

+/// This test checks that the warning includes the location in the C source
+/// file that contains the inline asm. Instead of saying <inline asm> for both.
+/// Although this warning is emitted in llvm it cannot be tested from IR as
+/// it does not have that location information at that stage.
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv7-m -c %s -o /dev/null \
+// RUN:   2>&1 | FileCheck %s
+
+// REQUIRES: arm-registered-target
+
+void bar(void) {
+    __asm__ __volatile__ ( "nop" : : : "sp");
+}
+
+// CHECK:      inline-asm-clobber-warning.c:12:28: warning: inline asm clobber list contains reserved registers: SP [-Winline-asm]
+// CHECK-NEXT:     __asm__ __volatile__ ( "nop" : : : "sp");
+// CHECK-NEXT:                            ^
+// CHECK-NEXT: inline-asm-clobber-warning.c:12:28: note: Reserved registers on the clobber list may not be preserved across the asm statement, and clobbering them may lead to undefined behaviour.

llvm/include/llvm/IR/LLVMContext.h

#undef LLVM_FIXED_MD_KIND
   /// with optional location information.  This function returns, so code should
   /// be prepared to drop the erroneous construct on the floor and "not crash".
   /// The generated code need not be correct.  The error message will be
   /// implicitly prefixed with "error: " and should not end with a ".".
   void emitError(unsigned LocCookie, const Twine &ErrorStr);
   void emitError(const Instruction *I, const Twine &ErrorStr);
   void emitError(const Twine &ErrorStr);
 
+  /// emitWarning - Emit a warning message to the currently installed warning
+  /// handler with location information. Message will be implicitly prefixed
+  /// with "warning: " and should not end with a ".".
+  void emitWarning(unsigned LocCookie, const Twine &WarningStr);
+
+  /// emitNote - Emit a note to the currently installed note handler with
+  /// location information. Same rules as emitWarning except the prefix will
+  /// be "note: ".
+  void emitNote(unsigned LocCookie, const Twine &NoteStr);
+
   /// Access the object which can disable optional passes and individual
   /// optimizations at compile time.
   OptPassGate &getOptPassGate() const;
 
   /// Set the object which can disable optional passes and individual
   /// optimizations at compile time.
   ///
   /// The lifetime of the object must be guaranteed to extend as long as the

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

if (InlineAsm::getKind(Flags) == InlineAsm::Kind_Clobber) {
       if (!TRI->isAsmClobberable(*MF, Reg))
         RestrRegs.push_back(Reg);
     }
     // Skip to one before the next operand descriptor, if it exists.
     I += InlineAsm::getNumOperandRegisters(Flags);
   }
 
   if (!RestrRegs.empty()) {
-    unsigned BufNum = addInlineAsmDiagBuffer(OS.str(), LocMD);
-    auto &SrcMgr = *MMI->getContext().getInlineSourceManager();
-    SMLoc Loc = SMLoc::getFromPointer(
-        SrcMgr.getMemoryBuffer(BufNum)->getBuffer().begin());
-
     std::string Msg = "inline asm clobber list contains reserved registers: ";
     ListSeparator LS;
     for (const Register &RR : RestrRegs) {
       Msg += LS;
       Msg += TRI->getName(RR);
     }
     const char *Note =
         "Reserved registers on the clobber list may not be "
         "preserved across the asm statement, and clobbering them may "
         "lead to undefined behaviour.";
-    SrcMgr.PrintMessage(Loc, SourceMgr::DK_Warning, Msg);
-    SrcMgr.PrintMessage(Loc, SourceMgr::DK_Note, Note);
+    MMI->getModule()->getContext().emitWarning(LocCookie, Msg.c_str());
+    MMI->getModule()->getContext().emitNote(LocCookie, Note);
   }
 
   emitInlineAsm(OS.str(), getSubtargetInfo(), TM.Options.MCOptions, LocMD,
                 MI->getInlineAsmDialect());
 
   // Emit the #NOAPP end marker.  This has to happen even if verbose-asm isn't
   // enabled, so we use emitRawComment.
   OutStreamer->emitRawComment(MAI->getInlineAsmEnd());

llvm/lib/IR/LLVMContext.cpp

void LLVMContext::diagnose(const DiagnosticInfo &DI) {
   if (DI.getSeverity() == DS_Error)
     exit(1);
 }
 
 void LLVMContext::emitError(unsigned LocCookie, const Twine &ErrorStr) {
   diagnose(DiagnosticInfoInlineAsm(LocCookie, ErrorStr));
 }
 
+void LLVMContext::emitWarning(unsigned LocCookie, const Twine &WarningStr) {
+  diagnose(DiagnosticInfoInlineAsm(LocCookie, WarningStr, DS_Warning));
+}
+
+void LLVMContext::emitNote(unsigned LocCookie, const Twine &NoteStr) {
+  diagnose(DiagnosticInfoInlineAsm(LocCookie, NoteStr, DS_Note));
+}
+
 //===----------------------------------------------------------------------===//
 // Metadata Kind Uniquing
 //===----------------------------------------------------------------------===//
 
 /// Return a unique non-zero ID for the specified metadata kind.
 unsigned LLVMContext::getMDKindID(StringRef Name) const {
   // If this is new, assign it its ID.
   return pImpl->CustomMDKindNames.insert(