Skip to content

[LTO] Improve diagnostics handling when parsing module-level inline assembly #75726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 18, 2023
Merged

[LTO] Improve diagnostics handling when parsing module-level inline assembly #75726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e5fb8b
[LTO] Improve diagnostics handling when parsing module-level inline a…
MaskRay Dec 10, 2023
09f2f27
Update lto-module-asm-err.ll
MaskRay Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/lib/CodeGen/CodeGenAction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,8 @@ void BackendConsumer::anchor() { }
} // namespace clang

bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo &DI) {
if (DI.getSeverity() == DS_Error)
HasErrors = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like the way the "HasErrors" bit works... can we refactor so every diagnostic handler doesn't need its own code to set the HasErrors bits? The way this is written, it looks like every LLVM-based compiler needs some code for this, not just clang.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created #75889 to improve this case a bit.

BackendCon->DiagnosticHandlerImpl(DI);
return true;
}
Expand Down
5 changes: 5 additions & 0 deletions clang/test/CodeGen/invalid_global_asm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
// REQUIRES: arm-registered-target
// RUN: not %clang_cc1 -emit-obj -triple armv6-unknown-unknown -o %t %s 2>&1 | FileCheck %s
// RUN: not %clang_cc1 -emit-obj -triple armv6-unknown-unknown -flto -o %t %s 2>&1 | FileCheck %s

/// Test the diagnostic behavior considering the whole system including the driver.
// RUN: not %clang --target=armv6-unknown-unknown -c -flto=thin -o %t %s 2>&1 | FileCheck %s
#pragma clang diagnostic ignored "-Wmissing-noreturn"
__asm__(".Lfoo: movw r2, #:lower16:.Lbar - .Lfoo");
// CHECK: <inline asm>:1:8: error: instruction requires: armv6t2
// CHECK-NOT: error:
8 changes: 3 additions & 5 deletions lld/test/MachO/lto-module-asm-err.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
; RUN: not %lld %t.bc -o /dev/null 2>&1 | FileCheck %s --check-prefix=REGULAR

;; For regular LTO, the original module name is lost.
;; TODO Fix the line number
; REGULAR: error: ld-temp.o <inline asm>:3:1: invalid instruction mnemonic 'invalid'
; REGULAR: error: <inline asm>:2:1: invalid instruction mnemonic 'invalid'

; RUN: opt -module-summary %s -o %t.bc
; RUN: not %lld %t.bc -o /dev/null 2>&1 | FileCheck %s --check-prefix=THIN
; RUN: not opt -module-summary %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=THIN

; THIN: error: {{.*}}.bc <inline asm>:2:1: invalid instruction mnemonic 'invalid'
; THIN: error: <inline asm>:2:1: invalid instruction mnemonic 'invalid'

target triple = "x86_64-apple-macosx10.15.0"
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/IR/DiagnosticHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class DiagnosticInfo;
/// which remarks are enabled.
struct DiagnosticHandler {
void *DiagnosticContext = nullptr;
bool HasErrors = false;
DiagnosticHandler(void *DiagContext = nullptr)
: DiagnosticContext(DiagContext) {}
virtual ~DiagnosticHandler() = default;
Expand Down
16 changes: 15 additions & 1 deletion llvm/lib/Object/ModuleSymbolTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "RecordStreamer.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalValue.h"
Expand Down Expand Up @@ -68,6 +69,11 @@ void ModuleSymbolTable::addModule(Module *M) {
static void
initializeRecordStreamer(const Module &M,
function_ref<void(RecordStreamer &)> Init) {
// This function may be called twice, once for ModuleSummaryIndexAnalysis and
// the other when writing the IR symbol table. If parsing inline assembly has
// caused errors in the first run, suppress the second run.
if (M.getContext().getDiagHandlerPtr()->HasErrors)
return;
StringRef InlineAsm = M.getModuleInlineAsm();
if (InlineAsm.empty())
return;
Expand Down Expand Up @@ -95,7 +101,8 @@ initializeRecordStreamer(const Module &M,
if (!MCII)
return;

std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(InlineAsm));
std::unique_ptr<MemoryBuffer> Buffer(
MemoryBuffer::getMemBuffer(InlineAsm, "<inline asm>"));
SourceMgr SrcMgr;
SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());

Expand All @@ -115,6 +122,13 @@ initializeRecordStreamer(const Module &M,
if (!TAP)
return;

MCCtx.setDiagnosticHandler([&](const SMDiagnostic &SMD, bool IsInlineAsm,
const SourceMgr &SrcMgr,
std::vector<const MDNode *> &LocInfos) {
M.getContext().diagnose(
DiagnosticInfoSrcMgr(SMD, M.getName(), IsInlineAsm, /*LocCookie=*/0));
});

// Module-level inline asm is assumed to use At&t syntax (see
// AsmPrinter::doInitialization()).
Parser->setAssemblerDialect(InlineAsm::AD_ATT);
Expand Down