[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-28 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td:230 def NullReturnedFromNonnullChecker : Checker<"NullReturnedFromNonnull">, HelpText<"Warns when a null pointer is returned from a function that has " An

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-28 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td:230 def NullReturnedFromNonnullChecker : Checker<"NullReturnedFromNonnull">, HelpText<"Warns when a null pointer is returned from a function that has " Sz

[PATCH] D53651: [clangd] Use thread pool for background indexing.

2018-11-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I've got a post-review comment about the use of SCHED_IDLE vs the needed gcc version. Comment at: clang-tools-extra/trunk/clangd/Threading.cpp:110 + T.native_handle(), + Priority == ThreadPriority::Low ? SCHED_IDLE : SCHED_OTHER, &priori

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

2018-12-10 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, A late question about this change. I notice that this change sometimes gives me additional DIFiles in the clang output compared to before. E.g. if I have a file /tmp/bar/foo.c containing just void foo() { } and I stand in /tmp/ and do clang -emit-llvm -S -g /tmp

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

2019-04-02 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed that with this commit I get a whole bunch (~40) of warnings like the below when compiling with gcc 7.4: [10/16] Building CXX object tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/clangTidyUtils.dir/HeaderGuard.cpp.o In file included from ../tool

[PATCH] D62476: [clangd] Support offsets for parameters in signatureHelp

2019-06-05 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/trunk/clangd/CodeComplete.cpp:925 +// FIXME: this should only be set on CK_CurrentParameter. +Signal.ContainsActiveParameter = true; + } Hi! gcc (7.4) warns on this code: ``` error: parameter

[PATCH] D62476: [clangd] Support offsets for parameters in signatureHelp

2019-06-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/trunk/clangd/CodeComplete.cpp:925 +// FIXME: this should only be set on CK_CurrentParameter. +Signal.ContainsActiveParameter = true; + } ilya-biryukov wrote: > ilya-biryukov wrote: > > uabelho

[PATCH] D67613: [DWARF-5] Support for DWARF-5 C++ language tags

2019-09-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D67613#1679879 , @xbolva00 wrote: > /srv/llvm-buildbot-srcatch/llvm-build-dir/clang-x86_64-debian-fast/llvm.src/tools/clang/tools/extra/modularize/Modularize.cpp:583:13: > warning: enumeration values 'lang_cxx_11' and 'lang_cxx

[PATCH] D67613: [DWARF-5] Support for DWARF-5 C++ language tags

2019-09-24 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D67613#1680176 , @uabelho wrote: > In D67613#1679879 , @xbolva00 wrote: > > > /srv/llvm-buildbot-srcatch/llvm-build-dir/clang-x86_64-debian-fast/llvm.src/tools/clang/tools/extra/modulariz

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

2020-06-04 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1034 const PreambleData &Preamble; - const PreamblePatch &Patch; + llvm::Optional Patch; llvm::StringRef Contents; Hi! When compiling with gcc 7.4 I see a warning that I

[PATCH] D81408: [builtins] Improve compatibility with 16 bit targets

2020-06-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Our out-of-tree target also have 16 bit ints and we've had to change a whole bunch of __builtin_clz to clzsi and __builtin_ctz to ctzsi to make things work so this patch is a step in the right direction for us too. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

2020-06-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:242 llvm::IntrusiveRefCntPtr - getFileSystem() const override { + getFileSystem(llvm::NoneType) const override { return VFS; A warning here too ``` 23:51:14

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

2020-06-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/clangd/support/FSProvider.h:39 + virtual llvm::IntrusiveRefCntPtr + getFileSystem(PathRef CWD) const; }; I noticed that gcc (7.4) warns on this line: ``` /data/repo/master/clang-tools-extra/clangd/s

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

2020-06-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi! I see a bunch of failures when I run libcxx testcases with this patch. Should there be "noexcept" on a number of more places? Failed Tests (8): libc++ :: libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp libc++ :: std/experiment

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

2020-06-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D82029#2109167 , @lxfind wrote: > Test failures are being fixed in https://reviews.llvm.org/D82338/new/ Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82029/new/ https:/

[PATCH] D81920: [clangd] Change FSProvider::getFileSystem to take CurrentWorkingDirectory

2020-06-24 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D81920#2109901 , @kadircet wrote: > Can you check if you see the warning with different versions of gcc? Originally I got them with 7.4.0 and now I tried with 9.3.0 and got them there as well. Repository: rG LLVM Github M

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

2020-05-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi @mibintc , I think I'm seeing some oddities with this patch (current trunk, 0054c46095 ). With clang -S -Xclang -emit-llvm bbi-42553.c -o - on the input float rintf(float x); #pragma STDC FE

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

2020-05-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. It seems to be this change in SemaStmt.cpp that makes the FENV-pragma have some effect regardless of the warning saying that the pragma doesn't have any effect: index aa0d89ac09c3..f76994a6dab3 100644 @@ -394,6 +394,11 @@ StmtResult Sema::ActOnCompoundStmt(SourceLo

[PATCH] D79510: [PATCH] When pragma FENV_ACCESS is ignored do not modify Sema.CurFPFeatures

2020-05-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Great! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79510/new/ https://reviews.llvm.org/D79510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

2022-08-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/test/Frontend/sarif-diagnostics.cpp:8 +// Omit filepath to llvm project directory +// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:444 +OpenMPClauseKind CK = static_cast(Type); +switch (CK) { +case OMPC_unknown: Hi @koops , clang complains on this patch with ``` ../../clang/lib/Basic/OpenMPKinds.cpp:444:13

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D123235#4656427 , @koops wrote: > To avoid build error in ppc64 adding a "default" to switch statement. I don't think it has anything particular to do with ppc64. I see the warning/error when i compile with clang 15 on a RHEL

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-08 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Btw, a bit confusing (to me at least) to reopen this review since it's already landed. Comment at: clang/lib/Basic/OpenMPKinds.cpp:450 +case OMPC_unknown: +default: + return "unknown"; Adding "default:" here silences the w

[PATCH] D122271: [Clang] -Wunused-but-set-variable warning - handle also pre/post unary operators

2022-03-28 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed these to warning when compiling libunwind/src/UnwindLevel1.c with this patch: 00:22:48 /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/sdk_1_20_ki_dev_test/libunwind/src/UnwindLevel1.c:175:12: warning: variable 'framesWalked' set but not used [-Wunuse

[PATCH] D122471: [IR] Require intrinsic struct return type to be anonymous

2022-03-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, No idea what is happening yet but I've seen some pretty nasty memory consumption by opt that I bisected to this patch. Any idea what could be going on? I'll see if I can see if I can extract some reproducer. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D122471: [IR] Require intrinsic struct return type to be anonymous

2022-03-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D122471#3416528 , @uabelho wrote: > Hello, > > No idea what is happening yet but I've seen some pretty nasty memory > consumption by opt that I bisected to this patch. > > Any idea what could be going on? > > I'll see if I can

[PATCH] D122471: [IR] Require intrinsic struct return type to be anonymous

2022-03-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D122471#3416797 , @nikic wrote: > @uabelho Can you please check whether > https://github.com/llvm/llvm-project/commit/d6887256c2cae1b1b721bd47459be6d86003db6f > fixes the problem you're seeing? Hi @nikic Yes, that patch sol

[PATCH] D129242: [clang-repl] Add host exception support check utility flag.

2022-07-28 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, The new testcase added in this patch fails for me all the time. I'm compiling on a linux RHEL 7 machine. When I run the clang-repl command in the test I get JIT session error: Symbols not found: [ _ZTIPKc ] error: Failed to materialize symbols: { (main, { _Z14

[PATCH] D129242: [clang-repl] Add host exception support check utility flag.

2022-07-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D129242#3686610 , @sunho wrote: > @uabelho Hi, that's a typeinfo symbol of libc++ abi. It's quite weird that > __cxa_throw is availble while _ZTIPKc is not. Do you have some special > setting regarding libc++ in your environm

[PATCH] D128158: [AMDGPU] Add amdgcn_sched_group_barrier builtin

2022-07-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:314 + +bool SchedGroup::isFull() const { + return MaxSize && Collection.size() >= *MaxSize; Compiling with gcc, I get a warning that this function is unused. I'm wondering, ther

[PATCH] D128569: Start support for HLSL `RWBuffer`

2022-07-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, Compiling with gcc I get the following warning with this patch: [1832/2330] Building CXX object tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/HLSLExternalSemaSource.cpp.o In file included from ../../clang/include/clang/Sema/ExternalSemaSource.h:15,

[PATCH] D129242: [clang-repl] Add host exception support check utility flag.

2022-07-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D129242#3687968 , @sunho wrote: > https://reviews.llvm.org/D130788 this is the patch fyi. Great, the patch works for us. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

2021-07-15 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, When compiling with gcc I see the following new warnings with this patch: [4/22] Building CXX object tools/clang/lib/StaticAnalyzer/Core/CMakeFiles/obj.clangStaticAnalyzerCore.dir/RangeConstraintManager.cpp.o /repo/uabelho/master-github/clang/lib/StaticAnalyzer

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

2021-07-15 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D105692#2879983 , @martong wrote: > In D105692#2879186 , @uabelho wrote: > >> Hi, >> >> When compiling with gcc I see the following new warnings with this patch: >> >> [4/22] Building

[PATCH] D119479: [clang][extract-api] Add global record support

2022-03-17 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, Did you see this sanitizer error when running global_record.c? https://lab.llvm.org/buildbot/#/builders/5/builds/20765/steps/13/logs/stdio Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119479/new/ https://reviews.l

[PATCH] D121678: [pseudo] Split greatergreater token.

2022-03-17 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, It seems like the new testcase fails when run with sanitizers: https://lab.llvm.org/buildbot/#/builders/5/builds/20833 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121678/new/ https://reviews.llvm.org/D121678

[PATCH] D119479: [clang][extract-api] Add global record support

2022-03-17 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D119479#3389684 , @zixuw wrote: > In D119479#3388498 , @uabelho wrote: > >> Hello, >> >> Did you see this sanitizer error when running global_record.c? >> >> https://lab.llvm.org/buildb

[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

2022-03-21 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Herald added a project: All. Comment at: clang/test/Driver/netbsd.c:477 +// RELOCATABLE-NOT: "-l +// RELOCATABLE-NOT: crt{{[^.]+}}.o Hi, I see this CHECK-NOT fail every now and then due to bad luck: 13:48:46 /repo/bbiswjenk/fem2s

[PATCH] D123300: [Clang] Enable opaque pointers by default

2022-04-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed this produces broken code: clang -cc1 -triple amdgcn-- -emit-llvm -o - -x c op.c with op.c being void bar(); void foo() { bar(); } The result is define dso_local void @foo() #0 { entry: call void @bar(i32 noundef 42) ret void }

[PATCH] D123300: [Clang] Enable opaque pointers by default

2022-04-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D123300#3451087 , @nikic wrote: > @uabelho The IR is correct, but requires using `opt -opaque-pointers` > explicitly. Normally, opaque pointer mode is automatically enabled, but there > is no explicit mention of `ptr` in the

[PATCH] D98110: [NFC][clangd] Use table to collect option aliases

2022-02-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:478 + HELP, METAVAR, VALUES) \ + {DriverID::OPT_##ID, DriverID::OPT_##ALIAS, (void *)ALIASARGS}, +#include "clang/Driver/Options.inc" -

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D124658#3495973 , @steakhal wrote: > This patch triggers a crash with this minimized example. > assertion at L205: `"The result operation type must have at least the same > number of bits as its operands."` > [...] > Please in

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Not sure if I'm doing something wrong but I can't apply this patch on current top of tree (a4190037fac0 ) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124658/new/ https://reviews.llvm.org/D1

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D124658#3500467 , @steakhal wrote: > [...] > Could you please give this another shoot at this @uabelho? Yes, the case that crashed for me works with the updated patch. Haven't done other testing. CHANGES SINCE LAST ACTIO

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

2022-05-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed that this patch isn't NFC. If I run clang-tidy -checks='*' empty.c -- on an empty file empty.c I get the following printouts with this patch: 'addr=address' 'arr=array' 'attr=attribute' 'buf=buffer' 'cl=client' 'cnt=count' 'col=column'

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

2022-05-11 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D124341#3504427 , @njames93 wrote: > In D124341#3502659 , @uabelho wrote: > >> Hi, >> >> I noticed that this patch isn't NFC. > > Whoops, good catch. I left in some debugging code, fixe

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

2022-05-12 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed when compiling with gcc 9.3.0 that we get a bunch of new warnings with this patch: [1/351] Building CXX object tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/MicrosoftCXXABI.cpp.o ../../clang/lib/AST/MicrosoftCXXABI.cpp:57:12: warning: 'virtual unsig

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

2022-05-13 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D122734#3509187 , @yaxunl wrote: > In D122734#3509096 , @yaxunl wrote: > >> In D122734#3508294 , @uabelho >> wrote: >> >>> Hi, >>> >>> I notic

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

2022-05-18 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Herald added a project: All. Hello, I wrote an issue about a crash with this patch: https://github.com/llvm/llvm-project/issues/55546 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114887/new/ https://reviews.llvm.org/D114

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I'm seeing what I think is a miscompile with this: opt -S -dse -o - dse.ll on @x = global [10 x [10 x i16]] zeroinitializer, align 1 define i16 @f() { entry: br label %do.body do.body: ; preds = %if.end,

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D87163#2270860 , @fhahn wrote: > In D87163#2270568 , @uabelho wrote: > >> Hi, >> >> I'm seeing what I think is a miscompile with this: > > Thanks for the reproducer. We have to be more ca

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D87163#2270918 , @uabelho wrote: > In D87163#2270860 , @fhahn wrote: > >> In D87163#2270568 , @uabelho wrote: >> >>> Hi, >>> >>> I'm seeing what I

[PATCH] D86547: [compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z

2020-08-26 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. We've run with changes like this for a long time to make the routines work for our out of tree target where int is also 16 bits so thumbs up from me even if I'm not sure I can formally LGTM it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-11-04 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi! I found a new problem after this was turned on again in 51ff04567b2f . With opt -S -o - bbi-49235.ll -memoryssa -gvn -dse -verify-memoryssa I hit opt: ../lib/Analysis/MemorySSA.cpp:2063: void

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-11-04 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D87163#2373580 , @uabelho wrote: > Hi! > > I found a new problem after this was turned on again in 51ff04567b2f > . I wrote https://bugs.llvm.org/show_bug.cg

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I'm seeing a miscompile with this patch. I'm not very good at the semantic differences between undef and poison so I don't know really where it goes wrong. Before Early CSE I see this in the input: entry: %cmp1 = icmp uge i16 1015, 16, !dbg !9 %shr = lshr

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D92270#2422625 , @aqjune wrote: > Hi, > > It seems it is related to two optimizations: > (1) `select i1 x, true, y` -> `or i1 x, y` > (2) `or i1 x, poison` -> `poison` > > Semantically, the first one is broken. It needs to freez

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D92270#2422741 , @aqjune wrote: > In D92270#2422661 , @uabelho wrote: > >> Should langref also be updated to describe this? Or does it already? >> I just found this >> "An instruction th

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend

2020-10-28 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. We got similar problems for our downstream target. We worked around that by bailing out early in PragmaSTDC_FENV_ACCESSHandler::HandlePragma for our target but perhaps one could check hasStrictFP() instead and ignore the pragma if that is false? Repository: rG LLVM

[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-09-20 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi @ychen, A whole bunch of libcxx testcases fail for me when I run with this patch: Failed Tests (90): libc++ :: libcxx/gdb/gdb_pretty_printer_test.sh.cpp libc++ :: std/numerics/rand/rand.adapt/rand.adapt.ibits/assign.pass.cpp libc++ :: std/numerics/rand/r

[PATCH] D94376: [MemCpyOpt] Enable MemorySSA by default

2021-02-22 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. I ran into a crash in memcpyopt with this patch. Running opt -S -o - bbi-53212.ll -memcpyopt yields opt: ../include/llvm/Support/Casting.h:104: static bool llvm::isa_impl_cl::doit(const From *) [To = llvm::MemoryUse, From = const llvm::MemoryUseOrDef *]: Assertion

[PATCH] D94376: [MemCpyOpt] Enable MemorySSA by default

2021-02-22 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D94376#2579242 , @nikic wrote: > @uabelho Thanks for the report! This issue should be resolved by > https://github.com/llvm/llvm-project/commit/4125afc35723b490556f7a38f7835f0914f70292. Yep, thanks! Repository: rG LLVM Git

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-06-21 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I'm seeing a failed assertion with this patch. Reproduce with clang --analyze bbi-57338.c Result: clang: /repo/uabelho/master-github/llvm/include/llvm/ADT/APSInt.h:148: bool llvm::APSInt::operator<(const llvm::APSInt &) const: Assertion `IsUnsigned == RHS.IsU

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-06-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, Another failed assertion that started appearing with this patch: clang --analyze bbi-57589.c which results in: clang: ../lib/Support/APInt.cpp:284: int llvm::APInt::compareSigned(const llvm::APInt &) const: Assertion `BitWidth == RHS.BitWidth && "Bit widths mu

[PATCH] D109506: [clangd] Print current request context along with the stack trace

2021-10-26 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, It seems like this patch may cause timeouts. I've seen it myself and also in this bot: http://lab.llvm.org:8011/#/builders/52/builds/11791 When I got the timeout in our downstream CI I think it was the Clangd :: crash.test that had problems. Repository: rG LLVM

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

2021-10-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, We're seeing a problem with this patch in our downstream (not public) buildbots. With an asan-built compiler we see the following: 10:08:55 FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (25832

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

2021-11-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D107049#3096807 , @v.g.vassilev wrote: > Can you share your cmake config line, the target triple and architecture? CC='/mycompiler/compiler-clang/bin/clang -march=corei7' CXX='/mycompiler/compiler-clang/bin/clang++ -march=co

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

2021-11-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. If we run without z3 it still fails, but in another way: [ RUN ] InterpreterTest.CatchException JIT session error: Symbols not found: [ __gxx_personality_v0, _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, __cxa_begin_catch, __

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

2021-11-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D107049#3099941 , @v.g.vassilev wrote: > Can you also paste the configure output of cmake? Attached in file.txt F20006228: file.txt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

2021-11-02 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D107049#3100939 , @lhames wrote: > In D107049#3096727 , @uabelho wrote: > >> Hi, >> >> We're seeing a problem with this patch in our downstream (not public) >> buildbots. With an asan-

[PATCH] D136090: Handle errors in expansion of response files

2022-11-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, I'm wondering if this is really working as expected/intended. With this patch, if I do clang empty.c @atfileesc.txt -dry-run on an empty file empty.c and atfileesc.txt containing just -I @name-with-at/ then I get the following error cannot open file: /r

[PATCH] D130510: Missing tautological compare warnings due to unary operators

2022-09-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D130510#3821641 , @aaron.ballman wrote: > In D130510#3817148 , @ebevhan wrote: > >> Hi! A bit of late feedback on this patch. We found a failure in our >> downstream testing likely or

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

2022-10-03 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Herald added a project: All. I wrote https://github.com/llvm/llvm-project/issues/58123 about a crash that I bisected to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86844/new/ https://reviews.llvm.org/D86844

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

2022-12-19 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, The following starts crashing with this patch: clang -cc1 -analyze -analyzer-checker=core bbi-77010.c It crashes with bbi-77010.c:6:1: warning: non-void function does not return a value [-Wreturn-type] } ^ clang: ../../clang/lib/StaticAnalyzer/Core/Range

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

2022-12-19 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D112621#4004409 , @steakhal wrote: > In D112621#4004372 , @uabelho wrote: > >> Hi, >> >> The following starts crashing with this patch: >> >> clang -cc1 -analyze -analyzer-checker=cor

[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

2022-12-19 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, I noticed that the following crashes with this patch: clang -Weverything bbi-77071.c Result: clang-16: ../../clang/lib/Analysis/UnsafeBufferUsage.cpp:248: void (anonymous namespace)::DeclUseTracker::discoverDecl(const clang::DeclStmt *): Assertion `Defs.co

[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

2022-12-19 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D138253#4006197 , @NoQ wrote: > Also if you're compiling your code with `-Weverything` by default, I do > recommend explicitly disabling `-Wunsafe-buffer-usage` for at least a month > or so, until we land all the basic functi

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

2022-12-19 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D112621#4004586 , @steakhal wrote: > Fixed by f61a08b67f5c8b0202dd30821766ee029e880b7c > Yep, thanks! Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

2022-12-21 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. It crashes on this as well: void f(int a[]) { int b = {a[1]}; } with clang -cc1 foo.c -Weverything I get clang: ../../clang/lib/Analysis/UnsafeBufferUsage.cpp:233: void (anonymous namespace)::DeclUseTracker::claimUse(const clang::DeclRefExpr *): Asser

[PATCH] D136090: Handle errors in expansion of response files

2022-11-04 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D136090#3905845 , @sepavloff wrote: > Thank you for reporting the issue. It was fixed in fdab9f1203ee > . Thanks! Repository: rG LLVM Github Monorepo C

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-01-31 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. @jhuber6 I really have no idea about the fix, but I've run the linker-wrapper.c testcase with asan built binaries with this fix 500 times now without seeing any failures. Without the fix it fails in like 1 out of 10 runs so it surely looks like the fix does something g

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-01-31 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D142985#4093783 , @jhuber6 wrote: > If this fixes the issues on your side, please open a bug so it can be > backported. I wrote https://github.com/llvm/llvm-project/issues/60437 Repository: rG LLVM Github Monorepo CHANG

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-02-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho resigned from this revision. uabelho added a comment. In D142985#4096297 , @jhuber6 wrote: > In D142985#4095701 , @uabelho wrote: > >> I wrote >> https://github.com/llvm/llvm-project/issues/60437 > > Grea

[PATCH] D140547: Perform access checking to private members in simple requirement.

2023-01-13 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, When I run the new testcase clang/test/SemaCXX/invalid-requirement-requires-expr.cpp with ASAN binaries the test fails like FAIL: Clang :: SemaCXX/invalid-requirement-requires-expr.cpp (1 of 1) TEST 'Clang :: SemaCXX/invalid-requirement-req

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

2022-06-13 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed that on one of our downstream (not public) buildbots the clang/test/Driver/femit-dwarf-unwind.s test seems to fail rather randomly. So if I run clang -target x86_64-apple-macos11.0 -c ../clang/test/Driver/femit-dwarf-unwind.s -o foo.o llvm-objdump

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

2022-06-13 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/tools/driver/cc1as_main.cpp:323-329 + if (auto *A = Args.getLastArg(OPT_femit_dwarf_unwind_EQ)) { +Opts.EmitDwarfUnwind = +llvm::StringSwitch(A->getValue()) +.Case("always", EmitDwarfUnwindType::Always) +

[PATCH] D127630: [MC] Fix likely uninitialized memory bug

2022-06-13 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho accepted this revision. uabelho added a comment. This revision is now accepted and ready to land. As I haven't been able to reproduce the problem manually myself I can't confirm that this fixes the problem, but it does seem likely. Thanks! Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D123674: Clang-Repl Error Recovery Bug Fix

2022-05-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I noticed that the testcase Interpreter/execute.cpp starts failing with this patch when run/compiled with asan: Failed Tests (1): Clang :: Interpreter/execute.cpp Seen in the buildbot run https://lab.llvm.org/buildbot/#/builders/5/builds/24221 The run on th

[PATCH] D126198: [analyzer][NFCi] Annotate major nonnull returning functions

2022-06-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I see crashes like this with this patch: clang: ../../clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:94: clang::ento::PointerToMemberData::PointerToMemberData(const clang::NamedDecl *, llvm::ImmutableList): Assertion `D' failed. Anyht

[PATCH] D126198: [analyzer][NFCi] Annotate major nonnull returning functions

2022-06-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D126198#3549790 , @uabelho wrote: > Hi, > > I see crashes like this with this patch: > > clang: > ../../clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:94: > clang::ento::PointerToMemberData::Point

[PATCH] D126198: [analyzer][NFCi] Annotate major nonnull returning functions

2022-06-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. @steakhal : Thanks for the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126198/new/ https://reviews.llvm.org/D126198 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D126689: [IR] Enable opaque pointers by default

2022-06-02 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Is anyone here ever using llvm-stress? I noticed that with this patch, llvm-stress seems to start producing code that llc crashes on, and it crashes even if I run on older llc versions. One example: llc -o /dev/null stress.ll crashes with llc: ../lib/CodeGen/Select

[PATCH] D126689: [IR] Enable opaque pointers by default

2022-06-02 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D126689#3553292 , @nikic wrote: > @uabelho Here's a slightly cleaned up test case that does not use opaque > pointers: > > target triple = "x86_64-unknown-linux-gnu" > > define void @test(i1 %cond, <1 x i16>* %p) { >

[PATCH] D126536: [pseudo] Add grammar annotations support.

2022-06-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp:235 Out.Sequence.push_back({Chunk}); } I get a warning/error on this line with this commit: ``` 13:31:17 ../../clang-tools-extra/pseudo/lib/grammar/Gramma

[PATCH] D126536: [pseudo] Add grammar annotations support.

2022-06-09 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp:235 Out.Sequence.push_back({Chunk}); } hokein wrote: > uabelho wrote: > > I get a warning/error on this line with this commit: > > ``` > > 13:31:17 ../../

[PATCH] D126689: [IR] Enable opaque pointers by default

2023-10-17 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Herald added subscribers: gysit, Dinistro, bviyer, jplehr, Moerafaat, kmitropoulou, zero9178, StephenFan. Herald added a reviewer: dcaballe. Hi @nikic, I know I'm very late to the party with this, but I just noticed that since opaque pointers got turned on by default, t

[PATCH] D126689: [IR] Enable opaque pointers by default

2023-10-17 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. @nikic: Thanks, nothing to do there then even if I'm surprised. I'm not sure but I *think* that llvm-reduce may have some problems with this. For my out of tree target I recently saw a case where llvm-reduced crashed with llvm-reduce: ../tools/llvm-reduce/deltas/Reduc

[PATCH] D126689: [IR] Enable opaque pointers by default

2023-10-17 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D126689#4654126 , @nikic wrote: > In D126689#4654124 , @uabelho wrote: > >> @nikic: Thanks, nothing to do there then even if I'm surprised. >> >> I'm not sure but I *think* that llvm-re

[PATCH] D159486: Fix build error in CI.

2023-09-11 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi @zahiraam , I have a couple of downstream testcases that fail with this patch. Before > clang bbi-86364.c -lm -O3 > ./a.out passed but with the patch the assert in the program fails: a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' f

[PATCH] D151834: Include math-errno with fast-math

2023-09-11 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi @zahiraam , I have a couple of downstream testcases that fail with this patch. Before > clang bbi-86364.c -lm -O3 > ./a.out passed but with the patch the assert in the program fails: a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' f

  1   2   >