From nobody Sat May 28 21:35:11 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 155611B5AD6F; Sat, 28 May 2022 21:35:12 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4L9Zhc020cz3Bmq; Sat, 28 May 2022 21:35:12 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1653773712; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+TIf2uGMwliffjrHO2HdnYv+FRvDhypCCMiJGf5mQvI=; b=VajXqZkwYayzUUIOjbuwJlcLG8WsZkPshvCzrm7xAFiwa8hUXU+X6RBFuIth+hKaYpo127 byazcBDeOG2/5xNyJ3T/WcnusGaON/EVNvr6vvc5KaB5TOK/y8YR8GCWWh6Y3848EBR++t 2LGWur02sifskIUXV/1RrrXKc/7xrjcmdGl4y1fdLlrYF38m+EYgnNwXxyN7ZSb2h1QDja vgwhArzRd6iFQ/+I/7wQAqJpsCa85dcvF2TwNXqFQS0Khkge2A08vyHP3yLUedjImgmYXc CUMskAywVEhySwDsSmeiDPt1iHsdVXfCG/fenzaQi8YE9GYLZVH8LxfELjNbLA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D14781692F; Sat, 28 May 2022 21:35:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 24SLZBbf053838; Sat, 28 May 2022 21:35:11 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 24SLZBZ9053837; Sat, 28 May 2022 21:35:11 GMT (envelope-from git) Date: Sat, 28 May 2022 21:35:11 GMT Message-Id: <202205282135.24SLZBZ9053837@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Dimitry Andric Subject: git: 6a5eebc190ab - main - Apply clang fix for assertion failure building putty 0.77 on i386 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: dim X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 6a5eebc190ab98de98ed7977cbdee3218758376e Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1653773712; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+TIf2uGMwliffjrHO2HdnYv+FRvDhypCCMiJGf5mQvI=; b=WKSK3Z8Oth9sMZvg2KFz1ommC1y/3CYD668AqrXHs4ymhp8FouzU6Lj2t0nYzV4t8ACqNp WLMKX+S/l8JhCnaW3e2tncbTGcqFcb4YM8gPS4d/rHiQqqoSGZPDNCrbi5xB//J6cbkE7s ZTRGcfXQoGbbLqX5FNR2Yw8YIuEeckMSfkZtbZsf6FI8wyAMsCmfjUoTcoua4LhJl3thnV CzD3s8GaCRqBBIh9jqJcS9Ez78KW3unLzpVKf8JrRS5BnmZxIPxc5invjxul7lTm3xDrjr 48liulGmtCVpRUreV5mrAlJbTjwxnKT3H4MF9Eb8UtppWYvjN9jR4ZUX3vY2kA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1653773712; a=rsa-sha256; cv=none; b=X45WcitGbw5NH3SYfhd6HNC9LW95UTxr8nmhsODGvDQNNNWpXc63qmPIGamgJm8p0Qq1ou liybozgr83kxAmzuAxG0pCM0oT9fmYoMz2uFVeL4FBkI6maGSvnGJnDwNBPFwfK7Sk0fBF eTrjOUoUr3kgyOdEFLnrKIzswsu6tUo5xnhIZfk52hG92Rvy1OIykfefVxWdVz0eQSv3Cz lQ8KBAJtXfeYMyXZTkdWpri5ovNDDtY0sYnrzxG1AtJN4t/uC3Chdmk8pydoQRkdhPzVnR 0Wy6wGJYLUTzBzL69XnpGUrTp18AJZDu7ee/1D5Rq+PVjirzcVuaMCQpZFtLGg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by dim: URL: https://cgit.FreeBSD.org/src/commit/?id=6a5eebc190ab98de98ed7977cbdee3218758376e commit 6a5eebc190ab98de98ed7977cbdee3218758376e Author: Dimitry Andric AuthorDate: 2022-05-28 21:26:37 +0000 Commit: Dimitry Andric CommitDate: 2022-05-28 21:26:37 +0000 Apply clang fix for assertion failure building putty 0.77 on i386 Merge commit 45084eab5e63 from llvm git (by Arthur Eubanks): [clang] Fix some clang->llvm type cache invalidation issues Take the following as an example struct z { z (*p)(); }; z f(); When we attempt to get the LLVM type of f, we recurse into z. z itself has a function pointer with the same type as f. Given the recursion, Clang simply treats z::p as a pointer to an empty struct `{}*`. The LLVM type of f is as expected. So we have two different potential LLVM types for a given Clang type. If we store one of those into the cache, when we access the cache with a different context (e.g. we are/aren't recursing on z) we may get an incorrect result. There is some attempt to clear the cache in these cases, but it doesn't seem to handle all cases. This change makes it so we only use the cache when we are not in any sort of function context, i.e. `noRecordsBeingLaidOut() && FunctionsBeingProcessed.empty()`, which are the cases where we may decide to choose a different LLVM type for a given Clang type. LLVM types for builtin types are never recursive so they're always ok. This allows us to clear the type cache less often (as seen with the removal of one of the calls to `TypeCache.clear()`). We still need to clear it when we use a placeholder type then replace it later with the final type and other dependent types need to be recalculated. I've added a check that the cached type matches what we compute. It triggered in this test case without the fix. It's currently not check-clang clean so it's not on by default for something like expensive checks builds. This change uncovered another issue where the LLVM types for an argument and its local temporary don't match. For example in type-cache-3, when expanding z::dc's argument into a temporary alloca, we ConvertType() the type of z::p which is `void ({}*)*`, which doesn't match the alloca GEP type of `{}*`. No noticeable compile time changes: https://llvm-compile-time-tracker.com/compare.php?from=3918dd6b8acf8c5886b9921138312d1c638b2937&to=50bdec9836ed40e38ece0657f3058e730adffc4c&stat=instructions Fixes #53465. Reviewed By: rnk Differential Revision: https://reviews.llvm.org/D118744 PR: 264318 Reported by: mandree MFC after: 3 days --- contrib/llvm-project/clang/lib/CodeGen/CGBuilder.h | 5 ++- contrib/llvm-project/clang/lib/CodeGen/CGCall.cpp | 18 ++++++-- .../clang/lib/CodeGen/CodeGenTypes.cpp | 52 ++++++++++++++++++---- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/contrib/llvm-project/clang/lib/CodeGen/CGBuilder.h b/contrib/llvm-project/clang/lib/CodeGen/CGBuilder.h index 7c9f41e84eaf..06b2da146603 100644 --- a/contrib/llvm-project/clang/lib/CodeGen/CGBuilder.h +++ b/contrib/llvm-project/clang/lib/CodeGen/CGBuilder.h @@ -9,10 +9,11 @@ #ifndef LLVM_CLANG_LIB_CODEGEN_CGBUILDER_H #define LLVM_CLANG_LIB_CODEGEN_CGBUILDER_H -#include "llvm/IR/DataLayout.h" -#include "llvm/IR/IRBuilder.h" #include "Address.h" #include "CodeGenTypeCache.h" +#include "llvm/IR/DataLayout.h" +#include "llvm/IR/IRBuilder.h" +#include "llvm/IR/Type.h" namespace clang { namespace CodeGen { diff --git a/contrib/llvm-project/clang/lib/CodeGen/CGCall.cpp b/contrib/llvm-project/clang/lib/CodeGen/CGCall.cpp index a37ff8844e88..34f7a421c933 100644 --- a/contrib/llvm-project/clang/lib/CodeGen/CGCall.cpp +++ b/contrib/llvm-project/clang/lib/CodeGen/CGCall.cpp @@ -38,6 +38,7 @@ #include "llvm/IR/InlineAsm.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/Intrinsics.h" +#include "llvm/IR/Type.h" #include "llvm/Transforms/Utils/Local.h" using namespace clang; using namespace CodeGen; @@ -1056,10 +1057,19 @@ void CodeGenFunction::ExpandTypeFromArgs(QualType Ty, LValue LV, // Call EmitStoreOfScalar except when the lvalue is a bitfield to emit a // primitive store. assert(isa(Exp.get())); - if (LV.isBitField()) - EmitStoreThroughLValue(RValue::get(&*AI++), LV); - else - EmitStoreOfScalar(&*AI++, LV); + llvm::Value *Arg = &*AI++; + if (LV.isBitField()) { + EmitStoreThroughLValue(RValue::get(Arg), LV); + } else { + // TODO: currently there are some places are inconsistent in what LLVM + // pointer type they use (see D118744). Once clang uses opaque pointers + // all LLVM pointer types will be the same and we can remove this check. + if (Arg->getType()->isPointerTy()) { + Address Addr = LV.getAddress(*this); + Arg = Builder.CreateBitCast(Arg, Addr.getElementType()); + } + EmitStoreOfScalar(Arg, LV); + } } } diff --git a/contrib/llvm-project/clang/lib/CodeGen/CodeGenTypes.cpp b/contrib/llvm-project/clang/lib/CodeGen/CodeGenTypes.cpp index 4839e22c4b14..7a8a7c916473 100644 --- a/contrib/llvm-project/clang/lib/CodeGen/CodeGenTypes.cpp +++ b/contrib/llvm-project/clang/lib/CodeGen/CodeGenTypes.cpp @@ -25,9 +25,20 @@ #include "llvm/IR/DataLayout.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Module.h" + using namespace clang; using namespace CodeGen; +#ifndef NDEBUG +#include "llvm/Support/CommandLine.h" +// TODO: turn on by default when defined(EXPENSIVE_CHECKS) once check-clang is +// -verify-type-cache clean. +static llvm::cl::opt VerifyTypeCache( + "verify-type-cache", + llvm::cl::desc("Verify that the type cache matches the computed type"), + llvm::cl::init(false), llvm::cl::Hidden); +#endif + CodeGenTypes::CodeGenTypes(CodeGenModule &cgm) : CGM(cgm), Context(cgm.getContext()), TheModule(cgm.getModule()), Target(cgm.getTarget()), TheCXXABI(cgm.getCXXABI()), @@ -382,9 +393,6 @@ llvm::Type *CodeGenTypes::ConvertFunctionTypeInternal(QualType QFT) { RecordsBeingLaidOut.erase(Ty); - if (SkippedLayout) - TypeCache.clear(); - if (RecordsBeingLaidOut.empty()) while (!DeferredRecords.empty()) ConvertRecordDeclType(DeferredRecords.pop_back_val()); @@ -415,11 +423,29 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) { if (const RecordType *RT = dyn_cast(Ty)) return ConvertRecordDeclType(RT->getDecl()); - // See if type is already cached. - llvm::DenseMap::iterator TCI = TypeCache.find(Ty); - // If type is found in map then use it. Otherwise, convert type T. - if (TCI != TypeCache.end()) - return TCI->second; + // The LLVM type we return for a given Clang type may not always be the same, + // most notably when dealing with recursive structs. We mark these potential + // cases with ShouldUseCache below. Builtin types cannot be recursive. + // TODO: when clang uses LLVM opaque pointers we won't be able to represent + // recursive types with LLVM types, making this logic much simpler. + llvm::Type *CachedType = nullptr; + bool ShouldUseCache = + Ty->isBuiltinType() || + (noRecordsBeingLaidOut() && FunctionsBeingProcessed.empty()); + if (ShouldUseCache) { + llvm::DenseMap::iterator TCI = + TypeCache.find(Ty); + if (TCI != TypeCache.end()) + CachedType = TCI->second; + if (CachedType) { +#ifndef NDEBUG + if (!VerifyTypeCache) + return CachedType; +#else + return CachedType; +#endif + } + } // If we don't have it in the cache, convert it now. llvm::Type *ResultType = nullptr; @@ -797,7 +823,15 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) { assert(ResultType && "Didn't convert a type?"); - TypeCache[Ty] = ResultType; +#ifndef NDEBUG + if (CachedType) { + assert(CachedType == ResultType && + "Cached type doesn't match computed type"); + } +#endif + + if (ShouldUseCache) + TypeCache[Ty] = ResultType; return ResultType; }