[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

ChuanqiXu added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221 CGF.EmitBlock(RealSuspendBlock); + } else if (ForcestackStart) { +Builder.CreateCall( lxfind wrote: > ChuanqiXu wrote: > > ChuanqiXu wrote: > > > can we rewrite it into:

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221 CGF.EmitBlock(RealSuspendBlock); + } else if (ForcestackStart) { +Builder.CreateCall( ChuanqiXu wrote: > ChuanqiXu wrote: > > can we rewrite it into: > > ``` > > else if (Su

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. > Can't we did as inline comments? No, because it would have already been too late. SuspendExpr returns the result of __builtin_coro_resume(awaiter.await_suspend().address()), which is different from the result of awaiter.await_suspend(). We need to be able to control th

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

ChuanqiXu added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221 CGF.EmitBlock(RealSuspendBlock); + } else if (ForcestackStart) { +Builder.CreateCall( ChuanqiXu wrote: > can we rewrite it into: > ``` > else if (SuspendRet != nullptr &&

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

xiangzhangllvm added inline comments. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:101 /// Constant fold bitcast, symbolically evaluating it with DataLayout. /// This always returns a non-null constant, but it may be a /// ConstantExpr if unfoldable. clin

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

ChuanqiXu added a comment. In D98638#2630864 , @lxfind wrote: > That's a fair point. I agree that we have no guarantee these are the only two > cases. > It does seem to me that coroutine implementation somewhat relies on proper > lifetime markers so that

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

clin1 added inline comments. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:101 /// Constant fold bitcast, symbolically evaluating it with DataLayout. /// This always returns a non-null constant, but it may be a /// ConstantExpr if unfoldable. xiangzhangllv

[PATCH] D98726: [analyzer] Remove unnecessary TODO

RedDocMD updated this revision to Diff 331164. RedDocMD added a comment. Removed unnecessary include Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98726/new/ https://reviews.llvm.org/D98726 Files: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeli

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. > Then if we want to put the result of the await_suspend in the stack, I think > we can do it under CodeGen part only. It should be easy to judge the return > type of await_suspend and create a call to llvm.coro.forcestack to the return > value of await_suspend. We prob

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. > Then we need to answer the question: how can we **prove** that the result of > symmetric transfer and %gro are the **only** exceptions from the above rules. > Or how can we know the list of exceptions wouldn't get longer and longer in > the future? > > Then go back to

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

LuoYuanke added a comment. I can reproduce the regression. I'll help to fix it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93594/new/ https://reviews.llvm.org/D93594 ___ cfe-commits mailing list cfe-c

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

ChuanqiXu added a comment. In D98638#2630778 , @lxfind wrote: > What do you think is the fundamental problem, though? It is hard to give a formal description for the problem. Let me try to explain it. What I want to say here is about rules that decide wh

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

xiangzhangllvm added a comment. In D98757#2630844 , @LuoYuanke wrote: > Probably we need a .ll test case to for constant folding. Fold constant is done in CSE and SCCP which are both passes run in Clang (O2 )

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

LuoYuanke added a comment. > at clang/test/CodeGen/X86/amx_api.c Probably we need a .ll test case to for constant folding. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:108 + // We won't fold bitcast for tile type, becasue there is no way to + // assigne a tmm reg from

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. Well, I guess another potential solution is to force emitting lifetime intrinsics for this part of coroutine in the front-end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98638/new/ https://reviews.llvm.org/D98638 __

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

clin1 added inline comments. Comment at: llvm/lib/Analysis/ConstantFolding.cpp:101 /// Constant fold bitcast, symbolically evaluating it with DataLayout. /// This always returns a non-null constant, but it may be a /// ConstantExpr if unfoldable. API for this

[PATCH] D98688: [-Wcalled-once-parameter] Harden analysis in terms of block use

NoQ added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1729 + handleBlockThatIsGuaranteedToBeCalledOnce(const BlockDecl *Block) override { +Data.flushWarnings(Block, S); + } Do i understand correctly that you're relying on the order

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. > Here what I want to say is we **shouldn't** handle all the symmetric > transfer from the above analysis. And we shouldn't change the ASTNodes and > Sema part. We need to solve about the above pattern. It is not easy to give a > solution since user could implement symm

[PATCH] D98694: [-Wcalled-once-parameter] Fix false positives for cleanup attr

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98694/new/ https://reviews.llvm.org/D98694 ___ cfe-co

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng added a comment. > This still doesn't report that the multilib configuration came from GCC when > it succeeds, does it? I suppose that's not a deal-breaker, but it would be > nice to have. Would it be difficult to implement? There is some order issue is those function are executed be

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

xiangzhangllvm added a comment. In D98757#2630764 , @LuoYuanke wrote: > Would you add a test case for it? at clang/test/CodeGen/X86/amx_api.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98757/new/ https

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

LuoYuanke added a comment. Would you add a test case for it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98757/new/ https://reviews.llvm.org/D98757 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

xiangzhangllvm added inline comments. Comment at: clang/test/CodeGen/X86/amx_api.c:39 +void test_tile_init(short row, short col) { + __tile1024i c = {row, col, {1, 2, 3}}; + __tile_stored(buf, STRIDE, c); we usually write like this __tile1024i c = {row, col}; r

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

ChuanqiXu added a comment. In D98638#2630607 , @lxfind wrote: > In D98638#2628082 , @ChuanqiXu wrote: > >> I am a little confused about the first problem. Would it cause the program >> to crash? (e.g., we access th

[PATCH] D98685: [X86][AMX] Rename amx-bf16 intrinsic according to correct naming convention

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG320b72e9cd77: [X86][AMX] Rename amx-bf16 intrinsic according to correct naming convention (authored by yubing). Repository: rG LLVM Github Monorep

[clang] 320b72e - [X86][AMX] Rename amx-bf16 intrinsic according to correct naming convention

Author: Bing1 Yu Date: 2021-03-17T11:22:52+08:00 New Revision: 320b72e9cd77504054bd2c837149df2f2bd4c149 URL: https://github.com/llvm/llvm-project/commit/320b72e9cd77504054bd2c837149df2f2bd4c149 DIFF: https://github.com/llvm/llvm-project/commit/320b72e9cd77504054bd2c837149df2f2bd4c149.diff LOG:

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

xiangzhangllvm created this revision. xiangzhangllvm added reviewers: LuoYuanke, pengfei, LiuChen3, yubing. Herald added a subscriber: hiraditya. xiangzhangllvm requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. We won't fold

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

t-tye added a comment. I vaguely remember that clang needed to know what code object it was going to request as it used that to either validate other options, or change the format of other passed cc1 options. If that is true, then I am not sure the defaulting approach works as clang will not kn

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

atirit added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3683 +Style.BraceWrapping.AfterEnum; +bool isLineTooBig = (strlen(Right.TokenText.data()) + + Right.OriginalColumn) > Style.ColumnLimit; curdeius wro

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1115-1124 + // Currently defaults to 3 in AMDGPUBaseInfo.cpp + // Using that default lets clang emit IR for amdgcn when llvm has been built + // without that target, provided the user want

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

rjmccall added a comment. In D96033#2630585 , @rsmith wrote: > @rjmccall I'd like your input on this patch in particular, if you have time. I've been specifically avoiding paying any attention to this patch because it sounds like an enormous time-sink to

[PATCH] D98676: [WebAssembly] Finalize SIMD names and opcodes

tlively added inline comments. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:1243 // Prototype f64x2 conversions defm "" : SIMDConvert If they are finalized, can we delete these "prototype" comments here and > elsewhere? (not necessarily in this CL) Yes, th

[clang] f5030f1 - [AST] Suppress diagnostic output when generating code

Author: Stephen Kelly Date: 2021-03-17T01:30:22Z New Revision: f5030f1a8e4affef2ab92b3268292f46d0052fd5 URL: https://github.com/llvm/llvm-project/commit/f5030f1a8e4affef2ab92b3268292f46d0052fd5 DIFF: https://github.com/llvm/llvm-project/commit/f5030f1a8e4affef2ab92b3268292f46d0052fd5.diff LOG:

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem marked 2 inline comments as done. gulfem added inline comments. Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391 + +if (!TM.getTargetTriple().isArch64Bit()) + return false; + lebedev.ri wrote: > 1. But all tests are using `x86_64` trip

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem marked 4 inline comments as done. gulfem added inline comments. Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:32 + // If lookup table has more than one user, + // do not generate a relative lookup table. + if (!GV.hasOneUse()) hans w

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1115-1124 + // Currently defaults to 3 in AMDGPUBaseInfo.cpp + // Using that default lets clang emit IR for amdgcn when llvm has been built + // without that target, provided the user wants this co

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem updated this revision to Diff 331139. gulfem added a comment. 1. Add no_relative_lookup_table.ll test case that checks the cases where relative lookup table should not be generated 2. Add comments about dso_local check and single use check Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:4695 +/// afterwards on the stack. class CoroutineSuspendExpr : public Expr { friend class ASTStmtReader; lxfind wrote: > ChuanqiXu wrote: > > It looks strange for the change of `Coro

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. In D98638#2628082 , @ChuanqiXu wrote: > I am a little confused about the first problem. Would it cause the program to > crash? (e.g., we access the fields of coroutine frame after the frame gets > destroyed). Or it just wastes som

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

rsmith added a comment. @rjmccall I'd like your input on this patch in particular, if you have time. I'm nervous in general about the looming idea of declaration unloading, but the fact that it's been working in Cling for a long time gives me some confidence that we can do that in a way that's

[PATCH] D98676: [WebAssembly] Finalize SIMD names and opcodes

aheejin accepted this revision. aheejin added inline comments. This revision is now accepted and ready to land. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:1243 // Prototype f64x2 conversions defm "" : SIMDConverthttps://reviews.llvm.org/D98676/new/ https

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

t-tye added a comment. I have no opinion, just making an observation and defer to @kzhuravl . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98746/new/ https://reviews.llvm.org/D98746 ___ cfe-commits mail

[PATCH] D97358: [X86] Support amx-bf16 intrinsic.

xiangzhangllvm added a comment. +1 first, didn't see key problems. Comment at: clang/lib/Headers/amxintrin.h:326 +__DEFAULT_FN_ATTRS_BF16 +static void __tile_tdpbf16ps(__tile1024i *dst, __tile1024i src1, + __tile1024i src2) { yubing

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

jdoerfert added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1115-1124 + // Currently defaults to 3 in AMDGPUBaseInfo.cpp + // Using that default lets clang emit IR for amdgcn when llvm has been built + // without that target, provided the user wants this

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1115-1124 + // Currently defaults to 3 in AMDGPUBaseInfo.cpp + // Using that default lets clang emit IR for amdgcn when llvm has been built + // without that target, provided the user want

[PATCH] D93164: [AST] Add generator for source location introspection

nathanchance added a comment. In D93164#2630534 , @steveire wrote: > In D93164#2630203 , @nathanchance > wrote: > >> I am seeing a spew of errors after 19740652c4c4329e2b9e77f96e5e31c360b4e8bb >>

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

t-tye added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1115-1124 + // Currently defaults to 3 in AMDGPUBaseInfo.cpp + // Using that default lets clang emit IR for amdgcn when llvm has been built + // without that target, provided the user wants this cod

[PATCH] D93164: [AST] Add generator for source location introspection

steveire added a comment. In D93164#2630203 , @nathanchance wrote: > I am seeing a spew of errors after 19740652c4c4329e2b9e77f96e5e31c360b4e8bb > (what > appears to be the latest ver

[clang] a00d440 - [AST] Hide errors from the attempt to introspect nodes

Author: Stephen Kelly Date: 2021-03-16T23:46:31Z New Revision: a00d44012820e9ed2eba623dd61ca9cf5a2ce115 URL: https://github.com/llvm/llvm-project/commit/a00d44012820e9ed2eba623dd61ca9cf5a2ce115 DIFF: https://github.com/llvm/llvm-project/commit/a00d44012820e9ed2eba623dd61ca9cf5a2ce115.diff LOG:

[PATCH] D98296: [clang-tidy] Simplify readability checks to not need ignoring* matchers

steveire updated this revision to Diff 331130. steveire added a comment. Herald added a project: clang-tools-extra. Update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98296/new/ https://reviews.llvm.org/D98296 Files: clang-tools-extra/clang-ti

[PATCH] D98712: [Utils] Support lit-like substitutions in update_cc_test_checks

arichardson added inline comments. Comment at: llvm/utils/update_cc_test_checks.py:228 + '%t' : tempfile.NamedTemporaryFile().name, + '%S' : os.getcwd(), +} Shouldn't this be the directory containing the test? Repository: rG LLVM Github Monorep

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

JonChesterfield added a comment. Thanks. I'm going to wait for some of the rocm people to pass judgement too as this code path is shared with hip / opencl etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98746/new/ https://reviews.llvm.org/D9874

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

NoQ added inline comments. Comment at: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist:11413 - categoryDead store + categoryUnused code typeDead initialization check_namedeadcode.DeadStores Charusso wrote: > "Initialization was never use

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, it unbreaks the build and is not totally bananas. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98746/new/ https://reviews.llvm.org/D9

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We already did so for scoped locks acquired in the constructor, this change extends the

[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

steveire added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp:285 +#include "path/to/header2.h" +#include "path/to/header.h" + njames93 wrote: > steveire wrote: > > I still find it really confusing that the "single ins

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

JonChesterfield created this revision. JonChesterfield added reviewers: yaxunl, t-tye, kzhuravl, ronlieb, b-sumner. Herald added subscribers: kerbowa, pengfei, tpr, dstuttard, nhaehnle, jvesely. JonChesterfield requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wdng

[PATCH] D98745: [clang] Add fixit for Wreorder-ctor

njames93 created this revision. njames93 added reviewers: rsmith, sammccall, aaron.ballman. Herald added a subscriber: mgrang. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Create fix-it hints to fix the order of constructors. To

[PATCH] D98191: [flang][driver] Add support for `-fdebug-dump-symbols-sources`

kiranchandramohan added a comment. In D98191#2611694 , @tskeith wrote: > `-fget-symbols-sources` is not a debug option, it's intended for integrating > with IDEs like vscode. So I think the original name is better. Unlike the > "dump" options it actually

[PATCH] D98429: [clang-format] Add new option to clang-format: SpaceBeforeForLoopSemiColon

curdeius requested changes to this revision. curdeius added a comment. Could you point me to a style guide that formats the code in this way please? Comment at: clang/include/clang/Format/Format.h:2838 + /// If ``false``, spaces will be removed before semi-colons in for loop

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

luismarques added a comment. This still doesn't report that the multilib configuration came from GCC when it succeeds, does it? I suppose that's not a deal-breaker, but it would be nice to have. Would it be difficult to implement? Regarding the Windows test issue, aren't there other test cases

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Great idea! Comment at: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist:11413 - categoryDead store + categoryUnused code typeDead initialization

[PATCH] D93164: [AST] Add generator for source location introspection

nathanchance added a comment. I am seeing a spew of errors after 19740652c4c4329e2b9e77f96e5e31c360b4e8bb (what appears to be the latest version of this patch): $ cmake \ -G Ninja \ -DCMAKE_BUILD_TYPE=Release \ -DCMAK

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

NoQ created this revision. NoQ added reviewers: vsavchenko, xazax.hun, Szelethus, martong, baloghadamsoftware, Charusso, steakhal, balazske, ASDenysPetrov. Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet. NoQ requested review of this revision. This is a

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso added a comment. @NoQ, could you upstream it and move this idea forward please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.llvm.org/D69726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

NoQ added a comment. That's what i like to see! You can test this via `clang_analyzer_printState()`. Comment at: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:343 + if (!DRV.isEmpty()) { +Out << Sep << "Mutex destroys with unknown result:" << NL; +for (auto

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

flx created this revision. flx added reviewers: ymandel, aaron.ballman. Herald added a subscriber: xazax.hun. flx requested review of this revision. Herald added a project: clang-tools-extra. This allows users to be more precise and exclude a type in a specific namespace from triggering the check

[PATCH] D98726: [analyzer] Remove unnecessary TODO

NoQ added a comment. In D98726#2629722 , @steakhal wrote: > For example this code does not trigger any warning: > https://godbolt.org/z/aM4xe7 FTFY: https://godbolt.org/z/vT5c7s This test isn't very interesting though because the warning doesn't have an

[PATCH] D93095: Introduce -Wreserved-identifier

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Decl.h:82 +enum ReservedIdentifierReason { + StartsWithUnderscoreAtGlobalScope, + StartsWithDoubleUnderscore, Because this is not a scoped enum, should these enumerators get an `RIR_` pre

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

dexonsmith added a comment. In D97850#2629464 , @ilyakuteev wrote: > If a fix will be in ModuleManager and only for ModuleCache the problem with > symlinks and path will not affect it as ModuleCache is managed by clang and > we can rely on it. > I agree

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

nikic added a comment. @yubing In this case I would recommend building `sqlite3.c` from test-suite under `perf stat` and look at the `instructions` metric. For me the command looks like this: perf stat CLANG_BINARY -w -Werror=date-time -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_

[PATCH] D98622: [-Wcalled-once-parameter] Let escapes overwrite MaybeCalled states

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. LGTM! Do any of the high-level comments at the beginning of the file need to be updated to reflect this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D98429: [clang-format] Add new option to clang-format: SpaceBeforeForLoopSemiColon

HazardyKnusperkeks added a subscriber: curdeius. HazardyKnusperkeks added inline comments. Comment at: clang/docs/ReleaseNotes.rst:168 +- Option ``SpaceBeforeForLoopSemiColon`` has been added to control whether + spaces will be added before the semi-colons in for loops. --

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:4695 +/// afterwards on the stack. class CoroutineSuspendExpr : public Expr { friend class ASTStmtReader; ChuanqiXu wrote: > It looks strange for the change of `CoroutineSuspendExpr`

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:207 + + LLVM_DEBUG(QT.dump()); + Actually this is one of those debug prints that should be removed and remained in here by accident. ==

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

lxfind added a comment. In D98638#2628082 , @ChuanqiXu wrote: > It looks like there are two things this patch wants to do: > > 1. Don't put the temporary generated by symmetric-transfer on the coroutine > frame. > 2. Offer a mechanism to force some values

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

njames93 added a comment. In D98635#2629827 , @whisperity wrote: > @aaron.ballman Could you please help me understand this pre-merge buildbot? > It says that Debian failed, with the following message: > > [15/1038] Running lint check for sanitizer sourc

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

whisperity added a comment. @aaron.ballman Could you please help me understand this pre-merge buildbot? It says that Debian failed, with the following message: [15/1038] Running lint check for sanitizer sources... FAILED: projects/compiler-rt/lib/CMakeFiles/SanitizerLintCheck cd /mnt/disks

[PATCH] D98726: [analyzer] Remove unnecessary TODO

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I don't think the `TODO` is addressed. By checking the //git blame// quickly, there was no change committed to the `SmartPtrChecker` affecting the collaboration with the `MallocCh

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

njames93 added a comment. Again LGTM, but see what alex says. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:135-137 + for (const FileByteRange &FBR : Error.Message.Ranges) { +Diag << getRange(FBR); + } nit: Elide braces. ===

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

beanz added a comment. If you add `include(CMakeDetermineSystem)` to the top of the cache file, it should initialize `CMAKE_HOST_SYSTEM_PROCESSOR`, which also probably gives you a reasonable value to key off. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89942/new/ https://reviews.llv

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384 + // Pad the two strings to the longer length. + std::size_t BiggerLength = std::max(Str1.size(), Str2.size()); + SmallString<32> S1PadE{Str1}, S2PadE

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384 + // Pad the two strings to the longer length. + std::size_t BiggerLength = std::max(Str1.size(), Str2.size()); + SmallString<32> S1PadE{Str1}, S2PadE{St

[PATCH] D97713: [ASTMatchers] Add documentation for convenience matchers

aaron.ballman added a comment. Thank you for this, I think it's helpful documentation! I have a few questions. Given that LibASTMatchersReference.html is a generated document, will these changes get overwritten by running `dump_ast_matchers.py`? Would it be possible to use some form of markup i

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384 + // Pad the two strings to the longer length. + std::size_t BiggerLength = std::max(Str1.size(), Str2.size()); + SmallString<32> S1PadE{Str1}, S2PadE

[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

steakhal added a comment. I'm really sorry about being sooo picky about this patch. It's not my expertise and the change seems to address a corner-case, so we have to especially careful not introducing bugs. My concern is that I still don't understand why do we want to do anything with reinterp

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

curdeius added a comment. That's my question as well to be honest. I don't know what's the policy on that. Anyway, it will simplify the code a bit I think. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://reviews.llvm.org/D98433 __

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

whisperity marked an inline comment as not done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365 +StringRef S2) { + StringRef S1Prefix = S1.take_front(

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365 +StringRef S

[PATCH] D78652: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:481 + + assert(TargetIdx.hasValue() && "Matched, but didn't find index?"); + TargetParams[PassedParamOfThisFn].insert( aaron.ballman

[PATCH] D96355: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with optionally considering differently qualified types mixable

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:172 + /// Add the specified qualifiers to the common type in the Mix. + MixData operator<<(Qualifiers Quals) const { +SplitQualType Split = CommonType.

[PATCH] D94355: [Passes] Add relative lookup table converter pass

hans added a comment. Sorry for being unresponsive for a while, I got distracted by various bugs. I skimmed this and it's looking great. Just added a few nit picks. Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:32 + // If lookup table has more than one use

[PATCH] D98726: [analyzer] Remove unnecessary TODO

RedDocMD created this revision. RedDocMD added reviewers: NoQ, vsavchenko, steakhal. Herald added subscribers: ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. RedDocMD requested review of this revision. Herald adde

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:706 +UniqueBindPower({LType, RType})) { + // FIXME: Don't emit multiple combinations here either. + StringRef DiagText = "a cal

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

aaron.ballman added a comment. In D69560#2629616 , @whisperity wrote: > In D69560#2629555 , @aaron.ballman > wrote: > >> [...] so please hold off on landing it for a bit in case any of the other >> reviewers want

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

whisperity added a comment. In D69560#2629555 , @aaron.ballman wrote: > [...] so please hold off on landing it for a bit in case any of the other > reviewers want to weigh in. Due to how the patch itself only being useful in practice if all the pieces

[PATCH] D98724: Fix false negative in -Wthread-safety-attributes

aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The original implementation didn't fire on non-template classes when a base class was an instanti

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365 +StringRef S2) { + StringRef S1Prefix = S1.take_front(S1.size() - N), +S2Prefix = S2.take_f

[PATCH] D98712: [Utils] Support lit-like substitutions in update_cc_test_checks

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa80a33e8b553: [Utils] Support lit-like substitutions in update_cc_test_checks (authored by ggeorgakoudis). Repository: rG LLVM Github Monorepo CH

[clang] a80a33e - [Utils] Support lit-like substitutions in update_cc_test_checks

Author: Giorgis Georgakoudis Date: 2021-03-16T10:36:22-07:00 New Revision: a80a33e8b55393c060e51486cfd8085b380eb36d URL: https://github.com/llvm/llvm-project/commit/a80a33e8b55393c060e51486cfd8085b380eb36d DIFF: https://github.com/llvm/llvm-project/commit/a80a33e8b55393c060e51486cfd8085b380eb36

  1   2   3   >