[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

[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

[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 ```

[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

[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:

[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,

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

2023-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D151834#4645910 , @zahiraam wrote: > In D151834#4645866 , @uabelho wrote: > >> What's the way forward here? Revert if it's unclear what to do? > > We have a fix for it. By eod a PR

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

2023-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. What's the way forward here? Revert if it's unclear what to do? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151834/new/ https://reviews.llvm.org/D151834 ___ cfe-commits

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

2023-09-12 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D151834#4644692 , @zahiraam wrote: > [...] >> I think the issue comes from here: >> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L2365 > > This looks like it's happening only for -Oi

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

2023-09-12 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'

[PATCH] D159486: Fix build error in CI.

2023-09-12 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'

[PATCH] D158715: [Driver] Cleanup last vestiges of Minix / Contiki support

2023-09-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/lib/Lex/InitHeaderSearch.cpp:336 - case llvm::Triple::Minix: -AddGnuCPlusPlusIncludePaths("/usr/gnu/include/c++/4.4.3", -"", "", "", triple); brad wrote: > uabelho wrote: > >

[PATCH] D158715: [Driver] Cleanup last vestiges of Minix / Contiki support

2023-09-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/lib/Lex/InitHeaderSearch.cpp:336 - case llvm::Triple::Minix: -AddGnuCPlusPlusIncludePaths("/usr/gnu/include/c++/4.4.3", -"", "", "", triple); @brad : I think this was the last

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2023-08-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. I've no idea what's happening but it seems like linking compiler-rt/lib/memprof/tests/MemProfUnitTests fails with this commit: [17/17] Linking CXX executable compiler-rt/lib/memprof/tests/MemProfUnitTests FAILED: compiler-rt/lib/memprof/tests/MemProfUnitTests :

[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

2023-07-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532 + void addToFlowCondition(const Formula &); + LLVM_DEPRECATED("Use Formula version instead", "") void addToFlowCondition(BoolValue ); sammccall

[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

2023-07-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532 + void addToFlowCondition(const Formula &); + LLVM_DEPRECATED("Use Formula version instead", "") void addToFlowCondition(BoolValue ); sammccall

[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

2023-07-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532 + void addToFlowCondition(const Formula &); + LLVM_DEPRECATED("Use Formula version instead", "") void addToFlowCondition(BoolValue ); There are

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

2023-07-04 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Failed Tests (6): Clang-Unit :: Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/ComplexBooleanWithSomeNames Clang-Unit :: Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/Conjunction

[PATCH] D151938: [clang][index] NFCI: Make `CXFile` a `FileEntryRef`

2023-06-19 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/tools/libclang/CXFile.h:18 +inline CXFile makeCXFile(OptionalFileEntryRef FE) { + return CXFile(FE ? >getMapEntry() : nullptr); +} Gcc warns here: ``` ../../clang/tools/libclang/CXFile.h:18:50: warning: cast from

[PATCH] D148827: -fsanitize=function: support C

2023-06-15 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, A question: it seems like this messes with alignment on functions? So with input program al.c: __attribute__((aligned(64))) void alignedfn(void) { __asm("nop"); } void alignedfn2(void) { __asm("nop"); } int main(void){} if we compile with

[PATCH] D147731: [3/11][POC][Clang][RISCV] Add typedef of the tuple type and define tuple type variant of vlseg2e32

2023-05-22 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments. Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:384 Policy PolicyAttrs; + bool IsTuple = false; ```

[PATCH] D150352: [clang][dataflow] Don't analyze templated declarations.

2023-05-15 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, ../../clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:43:27: error: 'build' is deprecated: Use the version that takes a const Decl & instead [-Werror,-Wdeprecated-declarations] ControlFlowContext::build(, *FuncDecl.getBody(),

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

2023-05-15 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D149718#4342320 , @Manna wrote: > In D149718#4341384 , @uabelho wrote: > >> In D149718#4341052 , @uabelho >> wrote: >> >>> Compiling with

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

2023-05-15 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D149718#4341052 , @uabelho wrote: > Compiling with clang 15.0.5 I get the following warning/error with this patch: > > ../../clang/include/clang/Sema/ParsedAttr.h:705:18: error: explicitly > defaulted move assignment

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

2023-05-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Compiling with clang 15.0.5 I get the following warning/error with this patch: ../../clang/include/clang/Sema/ParsedAttr.h:705:18: error: explicitly defaulted move assignment operator is implicitly deleted [-Werror,-Wdefaulted-function-deleted] AttributePool

[PATCH] D146987: [Assignment Tracking] Enable by default

2023-04-03 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. We've also seen crashes with this patch, e.g. clang -cc1 -triple x86_64-unknown-linux -emit-obj -debug-info-kind=constructor -O1 bbi-80938.c crashes with clang: ../include/llvm/ADT/IntervalMap.h:1187: bool llvm::IntervalMap >::overlaps(KeyT, KeyT) const [KeyT =

[PATCH] D146003: [StandardInstrumentations] Verify function doesn't change if analyses are preserved

2023-03-21 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. The improved verification found this: https://github.com/llvm/llvm-project/issues/61574 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146003/new/ https://reviews.llvm.org/D146003

[PATCH] D136497: [Clang] support for outputs along indirect edges of asm goto

2023-02-27 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. I noticed that gvn-hoist doesn't handle this very nicely, I just wrote https://github.com/llvm/llvm-project/issues/61023 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136497/new/ https://reviews.llvm.org/D136497

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

2023-02-24 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hello, I noticed that if I compile the compiler with ubsan, then lots of Clangd tests start failing with this patch: Failed Tests (165): Clangd :: ast-no-range.test Clangd :: ast.test Clangd :: call-hierarchy.test Clangd :: check-lines.test Clangd

[PATCH] D137527: [C++20] [Modules] [ClangScanDeps] Add ClangScanDeps support for C++20 Named Modules in P1689 format (2/4)

2023-02-10 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, There seems to be something funny going on with the ClangScanDeps/P1689 .cppm testcase added in this patch. It fails randomly for me on top of tree (4ad8f7a189570

[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 > >

[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

[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

[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 ::

[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 *):

[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

[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

[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

[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

[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:

[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

[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:

[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] 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

[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:

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

2022-07-30 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

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

2022-07-30 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] 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,

[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

[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, {

[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] 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] 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] 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] 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

[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] 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:

[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

[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: >

[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.

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

2022-05-31 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

[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/

[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

[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

[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,

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

2022-05-10 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] 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

[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/

[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

[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] 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] 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

[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

[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

[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

[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

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

2022-03-18 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? >> >>

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

2022-03-18 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. 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/

[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] 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

[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

[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#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++

[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] 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] D103938: Diagnose -Wunused-value based on CFG reachability

2021-09-21 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++ ::

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

2021-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D108695#2999677 , @Szelethus wrote: > rGfb4d590a622f4031900516360c07ee6ace01c5e6 > should > sort this out! Yes now it works. Thank you! Repository:

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

2021-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D108695#2999432 , @steakhal wrote: > In D108695#2999378 , @uabelho wrote: > >> Hi @Szelethus >> A couple of tests fail for me on trunk with this patch > > Uh, that's my unittest :D >

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

2021-09-14 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi @Szelethus A couple of tests fail for me on trunk with this patch: Failed Tests (3): Clang-Unit :: StaticAnalyzer/./StaticAnalysisTests/FalsePositiveRefutationBRVisitorTestBase.UnSatAtErrorNodeDueToRefinedConstraintNoReport Clang-Unit ::

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-03 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. This patch makes several libcxx tests fail due to the new warning. See e.g. http://lab.llvm.org:8011/#/builders/60/builds/4632 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108696/new/ https://reviews.llvm.org/D108696

[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]

[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

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

2021-06-24 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

[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 ==

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

2021-04-27 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. I noticed that with this patch libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp fails: error: 'warning' diagnostics seen but not expected: File

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

2021-04-25 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D79714#2713610 , @xbolva00 wrote: > I pushed a fix to just disable this warning for googlemock/gtest. Unfortunately that fix breaks compiling llvm with clang8. I commented in

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

2021-04-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D79714#2711898 , @xbolva00 wrote: > Can you try this fix? > > diff --git a/libcxx/utils/libcxx/test/params.py > b/libcxx/utils/libcxx/test/params.py > index ddf277dea246..abf712e78a61 100644 > ---

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

2021-04-23 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, With this commit I get failures in the following testcases when building check-runtimes on trunk: Failed Tests (29): libc++ :: libcxx/debug/containers/db_sequence_container_iterators.pass.cpp libc++ :: libcxx/gdb/gdb_pretty_printer_test.sh.cpp libc++

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

2021-04-07 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. I wrote https://bugs.llvm.org/show_bug.cgi?id=49859 about a miscompile that started occuring when turning on memoryssa again in DSE. I suspect that licm doesn't keep memoryssa updated properly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

  1   2   >