[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

2020-02-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. No proofs, but for completeness, some amd numbers Comment at: clang/lib/Basic/Targets/X86.cpp:1811-1814 +// K6 +case CK_K6: +case CK_K6_2: +case CK_K6_3: K6 should have 32B cache line size Comment a

[PATCH] D75057: Syndicate, test and fix base64 implementation

2020-02-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision. lebedev.ri added inline comments. This revision now requires changes to proceed. Comment at: compiler-rt/lib/fuzzer/FuzzerUtil.cpp:14-16 + +#include "llvm/Support/Base64.h" + I don't believe we can do this. libfuzzer

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. SGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D73020#1857704 , @rsmith wrote: > Looks good if you change the error to a warning. I'm going to treat that as an implicit accept since the error got changed into a warning. Will commit this soon. Repository: rG LLVM Gi

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 246559. lebedev.ri added a comment. Rebased, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73020/new/ https://reviews.llvm.org/D73020 Files: clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaExprC

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D73020#1892245 , @rsmith wrote: > In D73020#1891368 , @lebedev.ri > wrote: > > > In D73020#1857704 , @rsmith wrote: > > > > > Looks good if yo

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 246575. lebedev.ri added a comment. Rebased, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73380/new/ https://reviews.llvm.org/D73380 Files: clang/include/clang/AST/Decl.h clang/include/clang/Sema/

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3dd5a298bfff: [clang] Annotating C++'s `operator new` with more attributes (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73380/new/

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-25 Thread Roman Lebedev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb8fdafe68ce2: [Sema] Perform call checking when building CXXNewExpr (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73020/new/ https

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

2020-03-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. IIUC `default(firstprivate)` is being added in openmp-5.1, it is not in openmp-4.5, so it should not be accepted in pre-openmp-5.1-mode (i.e. it should be diagnosed) Comment at: clang/lib/Sema/SemaOpenMP.cpp:56 + DSA_firstprivate = + 1 << 1 | 1

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

2020-03-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. This will break existing `.clang-tidy` configs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/ https://reviews.llvm.o

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

2020-03-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:38 +/// extensions. +inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; } + njames93 wrote: > A lot of configuration options for clang tid

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

2019-10-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done. lebedev.ri added a comment. In D69435#1725795 , @AntonBikineev wrote: > Does anybody have suggestions on this? Can you be more specific what the question is? Comment at: clang-tools-extra

[PATCH] D32164: [clang-tidy] misc-misplaced-widening-cast: Disable checking of implicit widening casts by default

2019-11-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Herald added subscribers: llvm-commits, gamesh411, Szelethus, rnkovacs. Herald added a project: LLVM. The doc wasn't updated :/ Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D32164/new/ https://reviews.llvm.org/D32164

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/lib/Format/Format.cpp:768 LLVMStyle.CommentPragmas = "^ IWYU pragma:"; + LLVMStyle.ConstStyle = FormatStyle::CS_Leave; LLVMStyle.CompactNamespaces = false; Based on code reviews, this should be `CS_West`.

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang/lib/Format/Format.cpp:768 LLVMStyle.CommentPragmas = "^ IWYU pragma:"; + LLVMStyle.ConstStyle = FormatStyle::CS_Leave; LLVMStyle.CompactNamespaces = false; MyD

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang Tidy's script

2019-11-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision. lebedev.ri added inline comments. Comment at: clang/test/Analysis/check_analyzer_fixit.py:1 +#!/usr/bin/env python +# This does work with python3? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/ https://

[PATCH] D69897: Add #pragma clang loop aligned

2019-11-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3135 + +This predicates all the array references inside the loop to be aligned. The aligned access to them can increase fetch time and increase the performance. + What does this act

[PATCH] D69897: Add #pragma clang loop aligned

2019-11-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D69897#1735961 , @Meinersbur wrote: > Please see my comments in D69900 as well. > > For the font-end side, I suggest the syntax `vectorize_assume_alignment(32)` > instead. We also need to de

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. As far as i can tell the builder does not add any debug info. Should it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69785/new/ https://reviews.llvm.org/D69785 ___ cfe-com

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

2019-11-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Land this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D67545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

2019-11-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This should have a llvm ir test in `llvm/test/transforms/instcombine` i think, not a clang test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70143/new/ https://reviews.llvm.org/D70143 __

[PATCH] D69897: Add #pragma clang loop vectorize_assume_alignment(n)

2019-11-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. To reword, if `vectorize_assume_alignment(32)` is *NOT* lowered via `CodeGenFunction::EmitAlignmentAssumption()` in clang frontend, but lowered into `LoopAttributes::VectorizeAssumeAlignment`, then once the alignment that was specified does not match reality, there wil

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D69145#1748716 , @poelmanc wrote: > In D69145#1715611 , @mgehre wrote: > > > Exactly due to the issue you are fixing here, we ended up disabling the > > complete check because we didn

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

2019-11-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Thank you for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70366/new/ https://reviews.llvm.org/D70366 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. In D67545#1749561 , @balazske wrote: > Ping. It should be accepted before I can land it. @aaron.ballman already accepted. Repository:

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } courbet wrote: > JonasToth wrot

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D69145#1750529 , @aaron.ballman wrote: > In D69145#1748733 , @lebedev.ri > wrote: > > > In D69145#1748716 , @poelmanc > > wrote: > > > > > I

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: mitchell-stellar, lebedev.ri. lebedev.ri added a comment. @poelmanc @mitchell-stellar I'm confused by the diff - did this land? Was the correct patch committed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70165/new/

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ah, so it *really* landed in rG06f3dabe4a2e85a32ade27c0769b6084c828a206 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70165/new/ https://reviews.llv

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D70165#1750606 , @mitchell-stellar wrote: > Yes, there was a failing unit test that had to be fixed. I reverted the first > commit and then committed a version that fixed the failing test. I mean, the commit message (incl

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } JonasToth wrote: > lebedev.ri w

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Thank you, looks good to me. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69145/new/ https://reviews.llvm.org/D69145

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D70165#1751402 , @mitchell-stellar wrote: > Oops, it looks like I mixed up this ticket with > https://reviews.llvm.org/D69238. Sorry about that. Would you like me to > revert both commits and then re-commit with the correc

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, erichkeane, rsmith, vsk. lebedev.ri added a project: LLVM. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, dexonsmith, mehdi_amini. Herald added projects: clang, Sanitizers. Implicit Conversion Sanitizer is *

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Thank you for taking a look! In D70539#1755121 , @erichkeane wrote: > I haven't looked at the tests because I don't terribly understand the > sanitizer IR (hopefully someone else can take a look), You can ignore the final IR

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2422 } else if (type->isIntegerType()) { -// Note that signed integer inc/dec with width less than int can't -// overflow because of promotion rules; we're just eliding a few steps here.

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 230469. lebedev.ri marked 7 inline comments as done. lebedev.ri added a comment. @erichkeane & @aaron.ballman thank you for taking a look. Addressed review notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D70603: Change while to do-while

2019-11-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Missing a test; please upload all patches with full context (`-U9`) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70603/new/ https://reviews.llvm.org/D70603 ___ cfe-comm

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70539/new/ https://reviews.llvm.org/D70539 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Thank you for the review! I'll hold this off for a bit in case anyone else wants to comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70539/new/ https://reviews.llvm.org/D70539 ___

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-27 Thread Roman Lebedev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9872ea4ed1de: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/decrement… (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-27 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 231237. lebedev.ri added a comment. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70539/new/ https://reviews.llvm.org/D70539 Files: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGExprScalar.cp

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

2019-11-27 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Temporarily reverted in rG62098354ab1c6c0dd2cecc8ac526d411bfe8aa20 - the assertion does not hold, need to remove it likely. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2019-12-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, NegatedMask}, nullptr, "aligned_result"); --

[PATCH] D71503: New warning on for-loops that never run because condition is false on the first iteration

2019-12-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Thank you for working on this! This seems to be missing tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71503/new/ https://reviews.llvm.org/D71503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

2019-12-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, Negated

[PATCH] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

2019-12-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.cpp:21 + +void UnsignedSubtractionCheck::registerMatchers(MatchFinder *Finder) { + const auto UnsignedIntType = hasType(isUnsignedInteger()); The check'

[PATCH] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

2019-12-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.cpp:21 + +void UnsignedSubtractionCheck::registerMatchers(MatchFinder *Finder) { + const auto UnsignedIntType = hasType(isUnsignedInteger()); sorenj wro

[PATCH] D71674: [clang-tools-extra] Fix linking dylib for LLVMFrontendOpenMP

2019-12-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG I think this is correct solution. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71674/new/ https://reviews.llvm.org/D71674 __

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

2020-03-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:38 +/// extensions. +inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; } + njames93 wrote: > lebedev.ri wrote: > > njames93 wrote: > > >

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

2020-03-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. FWIW i'm personally still unconvinced this is fixing a real problem Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:43-45 +llvm::errs() +<< "Using ',' as a file extension delimiter is deprecated. Please " +

[PATCH] D75726: [ConstExprPreter] Updated constant interpreter documentation

2020-03-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This file needs linewrapping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75726/new/ https://reviews.llvm.org/D75726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

2020-03-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. No further comments from me, thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75621/new/ https://reviews.llvm.org/D75621 ___

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. What's the use case here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/ https://reviews.llvm.org/D75714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D75768: Add a warning for builtin_return_address/frame_address with > 0 argument

2020-03-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision. lebedev.ri added a reviewer: aaron.ballman. lebedev.ri added a comment. This seems reasonable to me but i'll defer to other reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75768/new/ https://reviews.

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

2020-03-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D59214#1916596 , @sammccall wrote: > So if I understand the history here: > > - the `IsOMPStructuredBlock` bit on `Stmt` was added to implement > `getStructuredBlock()` > - after review, `getStructuredBlock()` doesn't use th

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

2020-03-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. d5edcb90643104d6911da5c0ff44c4f33fff992f , looking forward to seeing better error recovery. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59214/new/ https://reviews.

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

2020-03-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I may be wrong, but i suspect those failures aren't actually due to the fact that we pessimize optimizations with this change, but that the whole execution just fails. Can you try running test-suite locally? Do tests themselves actually pass, ignoring the question of t

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. This seems to be already handled by clang static analyzer? (`clang-analyzer-cplusplus.PlacementNew`) :19:5: warning: Storage provided to placement new is only 2 bytes, whe

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a reviewer: NoQ. lebedev.ri added a comment. Herald added a subscriber: Charusso. In D76229#1925371 , @f00kat wrote: > In D76229#1925360 , @lebedev.ri > wrote: > > > This seems to be already handle

[PATCH] D76283: [IRBuilder] Use preferred target type for len argument of memory intrinsic functions

2020-03-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. So what happens if i manually create such IR, or create such IR without using IRBuilder? It will again compile fine via SelectionDAG and will be miscompiled via GlobalISel, correct? Don't you then want to harden `-verify` to barf on such mismatch, too? Repository:

[PATCH] D76283: [IRBuilder] Use preferred target type for len argument of memory intrinsic functions

2020-03-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. llvm-dev might be a better place for that question, wider coverage. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76283/new/ https://reviews.llvm.org/D76283 ___ cfe-commits

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

2020-03-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D59214#1926978 , @sammccall wrote: > In D59214#1919183 , @lebedev.ri > wrote: > > > d5edcb90643104d6911da5c0ff44c4f33fff992f > >

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

2020-03-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This seems misplaced - why is this in clang and not LLVM? Comment at: clang/include/clang/Basic/TargetInfo.h:1192 + // Get the cache line size of a given cpu. This method switches over + // the given cpu and returns `0` if the CPU is not found. +

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

2020-03-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D74918#1931488 , @zoecarver wrote: > @lebedev.ri LLVM may be better, I'm not sure. If you feel strongly I can move > it. I do think that it makes much more sense somewhere closer to the backend > @jyknight I'm planning on

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1129 + // should get unique names. Use the hash of module name to get a unique + // identifier and this is a best effort. + if (getCodeGenOpts().UniqueInternalFuncNames && maybe

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

2020-03-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D75591#1904969 , @lebedev.ri wrote: > IIUC `default(firstprivate)` is being added in openmp-5.1, > it is not in openmp-4.5, so it

[PATCH] D73845: [Gnu toolchain] Look at standard GCC multilib/multiarch paths by default

2020-02-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. 1. Please clang-format your changes 2. Hurd stuff is not a NFC refactoring, but adding some new feature - tests missing, i think? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73845/new/ https://reviews.llvm.org/D73845

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. bump @rsmith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73020/new/ https://reviews.llvm.org/D73020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

2020-02-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. bump @rsmith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73380/new/ https://reviews.llvm.org/D73380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 242373. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. @rsmith Thank you for taking a look! Addressed all review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73020/new/ h

[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

2020-02-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 242388. lebedev.ri added a comment. Rebased, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73380/new/ https://reviews.llvm.org/D73380 Files: clang/include/clang/AST/Decl.h clang/include/clang/Sema/

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:3887-3891 if (!I.isPowerOf2()) { Diag(Arg->getExprLoc(), diag::err_alignment_not_power_of_two) << Arg->getSourceRange(); return; }

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 3 inline comments as done. lebedev.ri added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:3887-3891 if (!I.isPowerOf2()) { Diag(Arg->getExprLoc(), diag::err_alignment_not_power_of_two) << Arg->getSourceRange();

[PATCH] D73996: [Sema] Demote 'alignment is not a power of two' error into a warning

2020-02-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, erichkeane. lebedev.ri added a project: clang. lebedev.ri added a parent revision: D73020: [Sema] Perform call checking when building CXXNewExpr. As @rsmith notes in https://reviews.llvm.org/D73020#inline-672219 while that is c

[PATCH] D73996: [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 242622. lebedev.ri retitled this revision from "[Sema] Demote 'alignment is not a power of two' error into a warning" to "[Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning". lebedev.ri added a comment. Sca

[PATCH] D74081: [clang][Analysis] CallGraph: store the actual call `Expr*` in the CallGraphNode::CallRecord

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: NoQ, erichkeane. lebedev.ri added a project: clang. Herald added a subscriber: Charusso. Storing not just the callee, but the actual call may be interesting for some use-cases. In particular, D72362 wo

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:21 + +inline bool operator==(const CallGraphNode::CallRecord &LHS, + const CallGraphNode::CallRecord &RHS) { NoQ wrote: > lebedev.ri wrote:

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 242713. lebedev.ri marked 11 inline comments as done. lebedev.ri added a comment. @NoQ Thank you for taking a look! Rebased, addressed most of the review notes (some tests could be added), split callgraph changes into a separate review. Repository: rG

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 242725. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Fixup disclaimer printing, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72362/new/ https://reviews.llvm.org/D72362 File

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153 +// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 'bar' here: +// CHECK-NOTES: :[[@LINE-14]]:18: note: Frame #2: function 'bar' calls

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as not done. lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153 +// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 'bar' here: +// CHECK-NOTES: :[[@LIN

[PATCH] D74298: Honor -finline-functions and -finline-hint-functions at -O0

2020-02-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This clearly needs tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74298/new/ https://reviews.llvm.org/D74298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D73020: [Sema] Perform call checking when building CXXNewExpr

2020-02-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Bump @rsmith thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73020/new/ https://reviews.llvm.org/D73020 ___ cfe-commits mai

[PATCH] D73996: [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning

2020-02-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Bump @rsmith / @erichkeane thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73996/new/ https://reviews.llvm.org/D73996 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D74399: [Driver][RISCV] Add RedHat Linux RISC-V triple

2020-02-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. passing-by remark: tests missing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74399/new/ https://reviews.llvm.org/D74399 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

2020-02-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Herald added a subscriber: wuzish. I would think `contract` change can be separated from the rest of the changes, and therefore should be a separate review (to reduce noise)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D68049#1870401 , @tmsriram wrote: > In D68049#1870094 , @tmsriram wrote: > > > In D68049#1868623 , @MaskRay wrote: > > > > > > In D68049#186596

[PATCH] D74436: Change clang default to -ffp-model=precise

2020-02-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Herald added a subscriber: wuzish. Comment at: clang/docs/UsersManual.rst:1388 - * ``precise`` Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (``-ffp-contract=fast``). This i

[PATCH] D72644: [clang] Add -fignore-exceptions

2020-02-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This review omitted cfe-commits list Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72644/new/ https://reviews.llvm.org/D72644 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

2020-02-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Now this patch should be reverted. This patch also omitted cfe-commits lists. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73852/new/ https://reviews.llvm.org/D73852 ___ cf

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

2020-02-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Also, rG398b wasn't sent to cfe-dev lists. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73852/new/ https://reviews.llvm.org/D73852 ___ cfe-commits mailing list cfe-commits@

[PATCH] D74051: Move update_cc_test_checks.py tests to clang

2020-02-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LGTM, i guess Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74051/new/ https://reviews.llvm.org/D74051 ___

[Diffusion] rG0e3a48778408: PR12350: Handle remaining cases permitted by CWG DR 244.

2020-02-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: cfe-commits, rsmith, vsk, lebedev.ri. lebedev.ri added a comment. Post-commit review, although i'm not sure this is the commit that is the problem. BRANCHES master /clang/lib/AST/NestedNameSpecifier.cpp:485-487 Was this change intentional? It seems, http://lab

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 244354. lebedev.ri marked 16 inline comments as done and an inline comment as not done. lebedev.ri added a comment. Ping. @aaron.ballman thank you for taking a look! Addressed review notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:28 +// 2. it is immutable, the way it was constructed it will stay. +template class ImmutableSmartSet { + ArrayRef Vector; aaron.ballman wrote: > "Smart" is not

[PATCH] D74081: [clang][Analysis] CallGraph: store the actual call `Expr*` in the CallGraphNode::CallRecord

2020-02-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 244508. lebedev.ri added a comment. Herald added a subscriber: martong. Rebased, NFC. In D74081#1860932 , @NoQ wrote: > Looks great, thanks! Thank you for the review! Repository: rG LLVM Github Monorepo CHAN

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 244509. lebedev.ri marked 8 inline comments as done. lebedev.ri added a comment. In D72362#1874690 , @aaron.ballman wrote: > LGTM aside from some nits. Thank you for the review! Nits addressed. Repository: rG

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-02-13 Thread Roman Lebedev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG49bffa5f8b79: [clang-tidy] misc-no-recursion: a new check (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72362/new/ https://reviews

<    14   15   16   17   18   19   20   21   22   23   >