[PATCH] D42273: Add hasTrailingReturn AST matcher

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5904 +return F->hasTrailingReturn(); + return false; +} juliehockett wrote: > lebedev.ri wrote: > > There are no negative tests in the unittest that cover this false path.

[PATCH] D42273: Add hasTrailingReturn AST matcher

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5904 +return F->hasTrailingReturn(); + return false; +} aaron.ballman wrote: > lebedev.ri wrote: > > juliehockett wrote: > > > lebedev.ri wrote: > > > > There are no negati

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

lebedev.ri added inline comments. Comment at: test/std/utilities/meta/meta.type.synop/endian.pass.cpp:10 + +// UNSUPPORTED: c++98, c++03, c++11, c++14, c++1z + c++17, not c++1z At least grep says there are no c++1z left in the libc++ tests. https://reviews.llvm

[PATCH] D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions

lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8959 + if (!S.getLangOpts().CPlusPlus) { +if (const TypeOfExprType *TET = dyn_cast(RHS->getType())) + RHS = TET->getUnderlyingExpr()->IgnoreParenImpCasts(); Please also add a commen

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

lebedev.ri added inline comments. Comment at: test/clang-tidy/readability-unnecessary-intermediate-var.cpp:206 + auto test = 1; // Test +#ifdef INTERMITTENT_MACRO + return (test == 1); Tests are nice :) But please, add the test with assert-like macro.. https:

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

lebedev.ri added inline comments. Comment at: test/clang-tidy/readability-unnecessary-intermediate-var.cpp:206 + auto test = 1; // Test +#ifdef INTERMITTENT_MACRO + return (test == 1); tbourvon wrote: > lebedev.ri wrote: > > Tests are nice :) > > But please, ad

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

lebedev.ri added inline comments. Comment at: test/SemaCXX/constexpr-default-arg.cpp:3 + +// expected-no-diagnostics + Down the line, it won't be obvious *what* this testcase is checking. At the very least wrap it into `namespace rdar_problem_36505742 {}` https

[PATCH] D42887: [Driver] Add option to manually control discarding value names in LLVM IR.

lebedev.ri added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3269 -// Disable the verification pass in -asserts builds. + const bool IsAssertBuild = #ifdef NDEBUG This logic seems sidewards. If `NDEBUG` is specified, then it is `assert()`-les

[PATCH] D42887: [Driver] Add option to manually control discarding value names in LLVM IR.

lebedev.ri added inline comments. Comment at: test/Driver/clang_f_opts.c:522 +// RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-DISCARD-NAMES %s +// RUN: %clang -### -S -fno-discard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-NO-DISCAR

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

lebedev.ri abandoned this revision. lebedev.ri added a comment. Thank you all! After some thought, i have come to conclusion that it is best to start with something less controversial, and simpler. https://reviews.llvm.org/D31252 ___ cfe-commits ma

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

lebedev.ri added a comment. This warning complains about macros from system headers, e.g. `PTHREAD_MUTEX_INITIALIZER`: $ ninja -j1 -v [1/110] /usr/bin/cmake -E __run_iwyu --tidy=/usr/local/bin/clang-tidy --source=../src/librawspeed/common/DngOpcodes.cpp -- /usr/local/bin/clang++ -DDEBUG -

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

lebedev.ri added a comment. And a lot of warnings from code using googletest, highlight: ../src/librawspeed/metadata/ColorFilterArrayTest.cpp:56:1: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] TEST(ColorFilterArrayTestBasic, Constructor) { ^ googletest/g

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

lebedev.ri added a comment. Relevant https://reviews.llvm.org/D31370, https://reviews.llvm.org/D19201 https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118084: [CUDA, NVPTX] Pass byval aggregates directly

lebedev.ri added a comment. In D118084#3271110 , @tra wrote: > In D118084#3271073 , @jdoerfert > wrote: > >> @lebedev.ri wanted to teach SROA how to deal with dynamic indices before, >> IIRC. It seems to be gene

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

lebedev.ri added a comment. The patch's description still only says what is being added, but provides zero motivation as to why it does what it does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116203/new/ https://reviews.llvm.org/D116203 _

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri updated this revision to Diff 339757. lebedev.ri added a comment. @rsmith i was able to make some forward progress here. What do you think about this? (I haven't updated all tests because i don't want to waste hours doing that if this is still wrong) Repository: rG LLVM Github Mon

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri added a comment. In D100581#2723329 , @nathanchance wrote: > In D100581#2721430 , > @nickdesaulniers wrote: > >> Huh, maybe not: https://godbolt.org/z/PnE1fMGWo > > This difference has caused some con

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri added a comment. Also, from the reviewer list, it doesn't look like anyone with much familiarity with clang/clang diags actually participated in the review? I think this might be the point to revert and re-review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri added a comment. In D100581#2723505 , @Abpostelnicu wrote: > Also I don’t remember seeing this proposed on cfe dev mailing list. There is no such requirement. I don't recall that happening basically ever, actually. Repository: rG LLVM Gi

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri added a comment. In D100581#2723621 , @Abpostelnicu wrote: > In D100581#2723611 , @lebedev.ri > wrote: > >> In D100581#2723505 , @Abpostelnicu >> wrote: >> >

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri added a comment. In D100581#2723709 , @aeubanks wrote: > reverted > regarding having a reviewer who is knowledgable in clang diagnostics, I > assumed that george.burgess.iv was knowledgable and was happy with the change > after his comments,

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

lebedev.ri added a comment. Thank you for looking into this! This also fixes the problem of not being able to find `libomp`, right? Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, +

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri added a comment. ping @rsmith - any further thoughts here? In D100388#2709870 , @lebedev.ri wrote: > @rsmith i was able to make some forward progress here. > What do you think about this? > > (I haven't updated all tests because i don't want

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri added a subscriber: efriedma. lebedev.ri added a comment. ping @rsmith / @efriedma - if there are any further thoughts on the problem, i would love to hear them Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100388/new/ https://reviews.l

[PATCH] D116521: [CMake] Factor out config prefix finding logic

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Seems like a cleanup to me, and consistent with D116524 . Soft-accept, but only if D116524 is accepted first. Rep

[PATCH] D116935: [IRBuilder] Introduce folder using inst-simplify, use for Or fold.

lebedev.ri added a comment. In D116935#3232126 , @craig.topper wrote: > If I remember correctly, the Or and And folds for 0 and all 1s are there to > optimize bitfield codegen for the frontend. Are we losing that optimization > if this patch goes in be

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri added a comment. What's the status here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99152/new/ https://reviews.llvm.org/D99152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D115848: tidy-llvm

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Just use compilation database? https://github.com/llvm-mirror/clang-tools-extra/blob/1c8cadde7ea4ca20a449edcffe10d23b612fe5d6/clang-tidy/tool/run-clang-tidy.py#L100 Repositor

[PATCH] D116216: Prevent adding module flag - amdgpu_hostcall multiple times.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. test coverage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116216/new/ https://reviews.llvm.org/D116216 ___

[PATCH] D116216: Prevent adding module flag - amdgpu_hostcall multiple times.

lebedev.ri added a comment. In D116216#3208096 , @pvellien wrote: > The testcases related to this patch are already added via this patch > https://reviews.llvm.org/D112820. If this patch does not result in test changes, then there is no test coverage.

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

lebedev.ri added a comment. Some thoughts Comment at: clang/lib/CodeGen/CGStmt.cpp:796 + bool EmitBoolCondBranch = !C || !C->isOne(); + bool CondIsConst = C; const SourceRange &R = S.getSourceRange(); I think, if we really want to give this a name, perhaps

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

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. I strongly suggest you bring up this ongoing creep of `if (DestTy->isX86_AMXTy()) return false;` on llvm-dev. I strongly suggest you are covering up bugs in you backend/pass w

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

lebedev.ri added a comment. In D98757#2630961 , @xiangzhangllvm wrote: > In D98757#2630942 , @lebedev.ri > wrote: > >> I strongly suggest you bring up this ongoing creep of `if >> (DestTy->isX86_AMXTy()) return f

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

lebedev.ri added a comment. Once again, i suggest to bring this up on llvm-dev. 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-c

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

lebedev.ri added a comment. In D98757#2631036 , @xiangzhangllvm wrote: > In D98757#2631019 , @lebedev.ri > wrote: > >> Once again, i suggest to bring this up on llvm-dev. > > That is obvious, > Discuss what, can y

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

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

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

lebedev.ri accepted this revision. lebedev.ri added a comment. Thank you! Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391 + +if (!TM.getTargetTriple().isArch64Bit()) + return false; + gulfem wrote: > lebedev.ri wrote: > > gulfem wrote: > >

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri added a comment. I'm a little bit lost with all this AMX stuff. Could you please explain in normal human words, what does `__tile_loadd()` do? I.e. given void wrapper(__tile& dst, const void *base, int stride) { _tile_loadd(dst, const void *base, int stride); } which bytes fro

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D99152#2643821 , @LuoYuanke wrote: > @lebedev.ri, this patch is mainly for discussing the approach that Florian > proposed, so I d

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri added a comment. load instruction loads contigious bytes. If that is not what is AMX is trying to use it for, then it is being used incorrectly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D99121: [IR][InstCombine] IntToPtr Produces Typeless Pointer To Byte

lebedev.ri updated this revision to Diff 332600. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Herald added subscribers: cfe-commits, pengfei. Herald added a project: clang. Also update a few clang tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri added a comment. In D99152#2644171 , @LuoYuanke wrote: > In D99152#2644017 , @lebedev.ri > wrote: > >> load instruction loads >> contigious bytes. >

[PATCH] D99121: [IR][InstCombine] IntToPtr Produces Typeless Pointer To Byte

lebedev.ri added subscribers: t.p.northover, dblaikie. lebedev.ri added a comment. In D99121#2644223 , @nlopes wrote: > The pointee type in LLVM doesn't really matter. It's even supposed to > disappear one day after the migration is completed. > E.g., i8*

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 57. lebedev.ri retitled this revision from "[clang][Sema] Add diagnostics for implicit widening of multiplication result" to "[clang-tidy] Add check for implicit widening of multiplication result". lebedev.ri edited the summary of this revision. lebede

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. @fhahn Are you still at this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96418/new/ https://reviews.llvm.org/D96418 __

[PATCH] D99402: [AMDGPU][OpenMP] Add /include to the search path

lebedev.ri added a comment. Any chance you might be interested in fixing the more general problem, benefiting every clang openmp user, not just amdgpu? D55725 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99402/new

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri added a comment. In D99152#2655274 , @fhahn wrote: > In D99152#2649520 , @LuoYuanke wrote: > >> In D99152#2647681 , @fhahn wrote: >> >>> I can't see any `load <256

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

lebedev.ri added a comment. In D99152#2655357 , @fhahn wrote: > In D99152#2655304 , @lebedev.ri > wrote: > >> In D99152#2655274 , @fhahn wrote: >> >>> In D99152#2649520

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

lebedev.ri added a comment. Thank you! The changes here look about reasonable to me. I have not checked if there are some changes that were missed. Usage of `CMAKE_INSTALL_FULL_` is suspect to me. Are you sure those are correct? Comment at: compiler-rt/cmake/Modules/AddCompiler

[PATCH] D99565: [X86] Support replacing aligned vector moves with unaligned moves when avx is enabled.

lebedev.ri added a comment. In D99565#2657813 , @craig.topper wrote: > The only use I could really see for this is to prevent a developers code from > crashing when it’s distributed to someone else. For paranoia because it’s > possible you have a bug an

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri marked an inline comment as done. lebedev.ri added a comment. In D93822#2664157 , @aaron.ballman wrote: > The CI is showing build failures and there are some clang-tidy nits to be > addressed as well. Thank you for taking a look! =

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri created this revision. lebedev.ri added reviewers: efriedma, rjmccall, rsmith, erichkeane, nikic. lebedev.ri added a project: LLVM. Herald added a subscriber: lxfind. lebedev.ri requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits,

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

lebedev.ri created this revision. lebedev.ri added reviewers: efriedma, rjmccall, rsmith, erichkeane, aaron.ballman, nikic. lebedev.ri added a project: LLVM. lebedev.ri requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Same idea as D99790

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2314 + llvm::Align Alignment = + getNaturalTypeAlignment(ThisTy, /*BaseInfo=*/nullptr, + /*TBAAInfo=*/nullptr, arichardson wrote: > Shouldn't

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 334929. lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. Addressed review comments. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93822/new/ https://reviews.llvm.org/D93822 F

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

lebedev.ri added a comment. @efriedma thank you for taking a look! I agree that D99790 is basically guaranteed safe, and this "probably" isn't. In D99791#2667047 , @efriedma wrote: > This feels scary: the C standard tech

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D98070#2616042 , @lebedev.ri wrote: > Please also add a test with global `IgnoreMacros=1` and > `readability-function-cognitive-co

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 335169. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. @aaron.ballman thank you for taking a look! Addressing all review notes other than `ssize_t` and `static_cast<>()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:29 + // Is this: long r = int(x) * int(y); ? + // FIXME: shall we skip brackets/casts/etc? + auto *BO = dyn_cast(E); aaron.ba

[PATCH] D99501: ignore -flto= options recognized by GCC

lebedev.ri added a comment. Tests missing. This somehow wasn't sent to cfe-commits list Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99501/new/ https://reviews.llvm.org/D99501 ___ cfe-commits mailing li

[PATCH] D99893: [WIP] Replace std::forward & std::move by cast expressions during Sema

lebedev.ri added a comment. To be noted, this generally goes against the [current] spirit of the AST, those were calls in the source code, but they won't be represented as such any longer, which is likely not very expected by whatever tool that could be consuming the AST. Though, i'm not sure ho

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

lebedev.ri added a comment. It would be good for @rjmccall / @rsmith / etc to actually finish reviewing this and accept it. I would personally want to see the next patches - what changes are needed for llvm analysis, transforms? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

lebedev.ri planned changes to this revision. lebedev.ri added a comment. @rjmccall thank you for taking a look! In D99791#2670333 , @rjmccall wrote: > The last major conversation we had about this was this RFC I sent out about > five years ago: > > htt

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added a comment. I'll land this tomorrow if no one wants to object. As discussed in D99791 , this is an obvious, and legal, bugfix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99790/new/ https://review

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

lebedev.ri added a comment. In D80344#2673148 , @asmith wrote: > In D80344#2671157 , @lebedev.ri > wrote: > >> It would be good for @rjmccall / @rsmith / etc to actually finish reviewing >> this and accept it. >>

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG0aa0458f1429: [CGCall] Annotate `this` argument with alignment (authored by lebedev.ri). Changed prior to commit: https

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

lebedev.ri added a comment. Thank you! Some more thoughts. Comment at: clang/lib/CodeGen/CGStmt.cpp:801 + SourceLocToDebugLoc(R.getEnd()), + loopMustProgress(CondIsConst)); fhahn wrote: > lebedev.ri wrote: > > Now this doesn't

[PATCH] D100037: [clang][UBSan] Passing underaligned pointer as a function argument is undefined behaviour

lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, rsmith, efriedma, vsk, filcab, vitalybuka. lebedev.ri added projects: LLVM, clang, Sanitizers. Herald added subscribers: jansvoboda11, dexonsmith, dang. lebedev.ri requested review of this revision. Herald added subscribe

[PATCH] D100037: [clang][UBSan] Passing underaligned pointer as a function argument is undefined behaviour

lebedev.ri updated this revision to Diff 335872. lebedev.ri added a comment. Fix usage of clang function argument -> llvm function argument mapping reading. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100037/new/ https://reviews.llvm.org/D100037

[PATCH] D100037: [clang][UBSan] Passing underaligned pointer as a function argument is undefined behaviour

lebedev.ri added a comment. In D100037#2674610 , @rsmith wrote: >> This is somewhat related to the following RFC by @rjmccall: >> https://lists.llvm.org/pipermail/llvm-dev/2016-January/094012.html > > It looks to me like this direction is in conflict with

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

lebedev.ri updated this revision to Diff 335894. lebedev.ri added a comment. Herald added subscribers: jansvoboda11, dexonsmith, dang. Add an option to toggle this optimization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99791/new/ https://revie

[PATCH] D100037: [clang][UBSan] Passing underaligned pointer as a function argument is undefined behaviour

lebedev.ri updated this revision to Diff 335893. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Unguard the UBSan check under `-fstrict*`. In D100037#2674610 , @rsmith wrote: > I assume your intent here is to eventually add LLVM

[PATCH] D100057: Remove warning "suggest braces" for aggregate initialization of an empty class with an aggregate base class.

lebedev.ri added a comment. This patch says what it does, but not why it does what it does. Did the pattern become idiomatic so that we now want to not warn? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100057/new/ https://reviews.llvm.org/D100057

[PATCH] D97116: Reduce the number of attributes attached to each function

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:496 //function. - CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth)); + if (LargestVectorWidth) +CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(Lar

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Seems fine to me. Might be good for someone else (@aaron.ballman ?) to also take a look, not sure. Thank you. Comment at: clang/lib/CodeGen/CGStmt.cpp:801 +

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added a comment. In D99790#2677917 , @brooksmoses wrote: > As a heads up, I'm seeing segfaults on internal code as a result of this > change, as well as errors in Eigen's unalignedassert.cpp test (specifically, > this line asserts: > https://

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added a comment. In D99790#2678384 , @brooksmoses wrote: > In D99790#2677919 , @lebedev.ri > wrote: > >> In D99790#2677917 , @brooksmoses >> wrote: >> >>> As a h

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added a comment. In D99790#2680366 , @brooksmoses wrote: > In D99790#2678674 , @lebedev.ri > wrote: > >> In D99790#2678384 , @brooksmoses >> wrote: >> >>> In any

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri reopened this revision. lebedev.ri added a comment. Temporarily reverted in 6270b3a1eafaba4279e021418c5a2c5a35abc002 . Eagerly awaiting fix for that bug. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added a subscriber: aaron.ballman. lebedev.ri added a comment. ping @rsmith @rjmccall @aaron.ballman To spell this out explicitly: i'm afraid i'm not really sure how to fix this preexisting thunk irgen bug. I think it should be fixed in `CodeGenVTables::maybeEmitThunk()`, likely in `

[PATCH] D99565: [X86] Support replacing aligned vector moves with unaligned moves when avx is enabled.

lebedev.ri added a comment. I'm still uncomfortable with changing current status quo, even though i obviously don't get to cast the final vote here. One should not use aligned loads in hope that they will cause an exception to detect address misalignment. That's UBSan's job. `-fsanitize=undefin

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. K, thx. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst:31 + any macro arguments are ignored, even if they shoul

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:130 + StringRef TyAsString = + IndexExprType->isSignedIntegerType() ? "ssize_t" : "size_t"; + aaron.ballman wrote: > aaron.b

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 336981. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. @aaron.ballman thank you for taking a look! Addressed all review notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93822/new

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 336985. lebedev.ri added a comment. Hardcode triple/target in tests, should fix 32-bit CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93822/new/ https://reviews.llvm.org/D93822 Files: clang-tools-extra

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri created this revision. lebedev.ri added a reviewer: rsmith. lebedev.ri added a project: LLVM. Herald added subscribers: dexonsmith, cryptoad, mgorny, dschuff. Herald added a reviewer: alexshap. lebedev.ri requested review of this revision. Herald added subscribers: cfe-commits, sstefan1,

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 337214. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Address final nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93822/new/ https://reviews.llvm.org/D93822 Files: clang-t

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

lebedev.ri added a comment. @aaron.ballman thank you so much for the review! Comment at: clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:130 + StringRef TyAsString = + IndexExprType->isSignedIntegerType() ? "ssize_t" : "size_t"; + --

[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG46b8ea2fff90: [clang-tidy] Add check for implicit widening of multiplication result (authored by lebedev.ri). Repository: rG LLVM Github Monorepo

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

lebedev.ri added a comment. In D80344#2684450 , @tentzen wrote: > Hi, > (Last call for review!!) Please refer to https://llvm.org/docs/CodeReview.html > Is there any more comments? This review has lasted for more than a year now. > I believe I had ad

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri added inline comments. Comment at: clang/lib/AST/VTableBuilder.cpp:1155-1157 + // Override it. Note that there may or may not be a thunk already. + VTableThunks.erase(Idx); + VTableThunks.insert({Idx, *TI}); rsmith wrote: > Can you modif

[PATCH] D100509: Support GCC's -fstack-usage flag

lebedev.ri added a comment. Might you be interested in implementing [[ https://bugs.llvm.org/show_bug.cgi?id=44418 | `-Wstack-usage=` ]] diag as a follow-up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100509/new/ https://reviews.llvm.org/D1005

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri updated this revision to Diff 337567. lebedev.ri added a comment. @rsmith thank you for taking a look! This still feels pretty much like doing it while blindfolded. I've tried to store `GlobalDecl`, but i'm clearly missing something else, since there are still 23 test failures, and not

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri updated this revision to Diff 337679. lebedev.ri marked 10 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100388/new/ https://reviews.llvm.org/D100388 Files: clang/include/clang/AST/VTableBuilder.h clang/includ

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri abandoned this revision. lebedev.ri added a comment. You know, somehow i don't think me wasting 30 more hours on this to experimentally find a working permutation is productive. Comment at: clang/include/clang/Basic/Thunk.h:157 + + /// Holds a pointer to the overri

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri added a comment. I have tried to fix this in D100388 , but i don't quite understand all the moving parts, so it's still not in a working state (even though it fixed those two tests), so i'm not sure how productive it is to develop it via a review feedb

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

lebedev.ri added a comment. Patch description is empty. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100874/new/ https://reviews.llvm.org/D100874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D100879: [Clang] Propagate guaranteed alignment for malloc and others

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2060 +case Builtin::BIstrndup: + RetAttrs.addAlignmentAttr(Context.getTargetInfo().getNewAlign() / +Context.getTargetInfo().getCharWidth()); ---

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri updated this revision to Diff 313788. lebedev.ri added a comment. - Properly handle dependent types (bailout instead of crashing) - Add `ASTContext::getCorrespondingSignedType()` - Add fix-it for the `unsigned long r = int(x) * int(y)` case Repository: rG LLVM Github Monorepo CHANG

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

lebedev.ri added a comment. Do you plan to follow-up with the diagnostic itself, in some form? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76572/new/ https://reviews.llvm.org/D76572 ___ cfe-commits mai

<    17   18   19   20   21   22   23   >