[PATCH] D43590: [clang-format] Fix regression when getStyle() called with empty filename

bjope added inline comments. Comment at: unittests/Format/FormatTest.cpp:11727 +TEST(FormatStyle, GetStyleWithEmptyFileName) { + auto Style1 = getStyle("file", "", "Google"); + ASSERT_TRUE((bool)Style1); Do you really want to try to find a ".clang-format" file

[PATCH] D43732: Resolve build bot problems in unittests/Format/FormatTest.cpp

bjope created this revision. Herald added a subscriber: klimek. Make the new GetStyleWithEmptyFileName test case independent of the file system used when running the test. Since the test is supposed to use the fallback "Google" style we now use a InMemoryFileSystem to make sure that we do not acci

[PATCH] D43590: [clang-format] Fix regression when getStyle() called with empty filename

bjope added inline comments. Comment at: unittests/Format/FormatTest.cpp:11727 +TEST(FormatStyle, GetStyleWithEmptyFileName) { + auto Style1 = getStyle("file", "", "Google"); + ASSERT_TRUE((bool)Style1); bjope wrote: > Do you really want to try to find a ".clan

[PATCH] D43732: Resolve build bot problems in unittests/Format/FormatTest.cpp

bjope added a comment. This is an attempt to fix problems seen after https://reviews.llvm.org/D43590 Repository: rC Clang https://reviews.llvm.org/D43732 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D43732: Resolve build bot problems in unittests/Format/FormatTest.cpp

This revision was automatically updated to reflect the committed changes. Closed by commit rC326086: Resolve build bot problems in unittests/Format/FormatTest.cpp (authored by bjope, committed by ). Changed prior to commit: https://reviews.llvm.org/D43732?vs=135794&id=135891#toc Repository:

[PATCH] D43732: Resolve build bot problems in unittests/Format/FormatTest.cpp

bjope added a comment. In https://reviews.llvm.org/D43732#1020639, @benhamilton wrote: > Actually, looks like all the other tests explicitly want a non-empty > in-memory FS, so there's no good cleanup I can do. Well, it was only this new test case that failed, so I think other tests are fine

[PATCH] D53299: [Fixed Point Arithmetic] Fix for clang-tools-extra warning

bjope accepted this revision. bjope added a comment. This revision is now accepted and ready to land. Just some inline nit:s about whitespace. LGTM, apart from that! Comment at: clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:63 InitType->getAs()

[PATCH] D53707: [Fixed Point Arithmetic] Refactor fixed point casts

bjope created this revision. bjope added reviewers: leonardchan, ebevhan. - Added names for some emitted values (such as "tobool" for the result of a cast to boolean). - Replaced explicit IRBuilder request for doing sext/zext/trunc by using CreateIntCast instead. - Simplify code for emitting sat

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

bjope added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3208 + if (LHSWidth < CommonWidth) +LHS = LHSSign ? Builder.CreateSExt(LHS, CommonTy) + : Builder.CreateZExt(LHS, CommonTy); `LHS = Builder.CreateIntCast(LHS, Common

[PATCH] D53707: [Fixed Point Arithmetic] Refactor fixed point casts

This revision was automatically updated to reflect the committed changes. Closed by commit rC345398: [Fixed Point Arithmetic] Refactor fixed point casts (authored by bjope, committed by ). Changed prior to commit: https://reviews.llvm.org/D53707?vs=171113&id=171311#toc Repository: rC Clang

[PATCH] D57219: [Fixed Point Arithmetic] Fixed Point Comparisons

bjope added inline comments. Comment at: clang/test/Frontend/fixed_point_comparisons.c:2 +// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED +// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown

[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

bjope added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1223 if (SrcType->isFixedPointType()) { -if (DstType->isFixedPointType()) { - return EmitFixedPointConversion(Src, SrcType, DstType, Loc); -} else if (DstType->isBooleanType()) { +if (

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

bjope added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:112 +/// throw, if it's unknown or if it won't throw. +enum State Behaviour : 2; + JonasToth wrote: > bjope wrote: > > (post-commit comments) > > > > I've seen that @dyung did

[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

bjope added inline comments. Comment at: clang/test/Frontend/fixed_point_conversions.c:45 + +short _Accum sa_const9 = 2u; // DEFAULT-DAG: @sa_const9 = {{.*}}global i16 256, align 2 +unsigned short _Accum usa_const3 = 2; Perhaps we should verify that we do not me

[PATCH] D53701: [Analyzer] Instead of recording comparisons in interator checkers do an eager state split

bjope added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:825 + // not then create a new symbol for the offset. + SymbolRef Sym; + if (!LPos || !RPos) { This is an uninitialized version of `Sym` that will be used on line 835 and

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

bjope added subscribers: dyung, bjope. bjope added inline comments. Herald added a subscriber: Charusso. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:112 +/// throw, if it's unknown or if it won't throw. +enum State Behaviour : 2; + (post-commit comme

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

bjope added a subscriber: sammccall. bjope added a comment. Maybe @sammccall remembers why it was decided to rewrite the enum into constexpr:s in https://reviews.llvm.org/rCTE319608 ? Do we need a similar solution here? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

bjope added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5888 def note_typecheck_assign_const : Note< "%select{" Shouldn't there be one more entry here as well? (see comment about updating both err_typecheck_assign_const and note

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

bjope added a comment. This patch looks good to me now. But I'm currently working in the same out-of-tree project as the author, so it would be nice if someone else could have a look as well and accept this. https://reviews.llvm.org/D37254 ___ cfe-

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

bjope accepted this revision. bjope added a comment. This revision is now accepted and ready to land. Nobody else seems to bother. At least no objections, so I think we can land this now. https://reviews.llvm.org/D37254 ___ cfe-commits mailing list

[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

This revision was automatically updated to reflect the committed changes. Closed by commit rL313628: [Sema] Disallow assigning record lvalues with nested const-qualified fields. (authored by bjope). Changed prior to commit: https://reviews.llvm.org/D37254?vs=114340&id=115831#toc Repository:

[PATCH] D65233: driver: Don't warn about assembler flags being unused when not assembling; different approach

bjope added a comment. I made another fixup (similar to https://reviews.llvm.org/rL367176) here https://reviews.llvm.org/rL367182. Can someone please take a look (a post-commit review) of https://reviews.llvm.org/rL367182 to verify that I did not misunderstand the intention with the test someh

[PATCH] D65233: driver: Don't warn about assembler flags being unused when not assembling; different approach

bjope added a comment. In D65233#1603783 , @thakis wrote: > In D65233#1603741 , @bjope wrote: > > > I made another fixup (similar to https://reviews.llvm.org/rL367176) here > > https://reviews.llvm.org/rL367182. >

[PATCH] D63518: BitStream reader: propagate errors

bjope added inline comments. Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619 +// FIXME check that the enum is in range. +auto Code = static_cast(MaybeCode.get()); + This has caused problem for our sanitizer tests the last couple of day

[PATCH] D64262: Bitstream reader: Fix undefined behavior seen after rL364464

bjope created this revision. bjope added a reviewer: jfb. Herald added subscribers: kadircet, dexonsmith, ilya-biryukov. Herald added a project: clang. After rL364464 the following tests started to fail when running the clang-doc tests with an ubsan instrumente

[PATCH] D63518: BitStream reader: propagate errors

bjope added inline comments. Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619 +// FIXME check that the enum is in range. +auto Code = static_cast(MaybeCode.get()); + jfb wrote: > jfb wrote: > > bjope wrote: > > > This has caused problem

[PATCH] D64262: Bitstream reader: Fix undefined behavior seen after rL364464

This revision was automatically updated to reflect the committed changes. Closed by commit rL365239: Bitstream reader: Fix undefined behavior seen after rL364464 (authored by bjope, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: h

[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

bjope added inline comments. Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3270 + if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ)) +Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ); A is not used, so this does not compile when us

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

bjope added a comment. In https://reviews.llvm.org/D45619#1075089, @avt77 wrote: > It's terrible but my new test was failed again as result of commit of this > patch! > > ///b/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Frontend/ftime-report-template-decl.cpp:155:11: > error: e

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

bjope added a comment. I can't see that it has been reverted. But I guess that the table maybe is sorted based on time spent in each pass? So that is why it might be sorted differently on different buildbots (or when using pipe etc). So I think a quick fix is to add -DAG to the checks that can

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

bjope added a comment. buildbots have been failing all day (and were still failing). So I pushed my suggested fix. I hope that was OK. Repository: rL LLVM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

bjope added a comment. I tried running /clang -cc1 -O3 -funroll-loops -S -emit-llvm pragma-do-while-unroll.cpp -o - -mllvm -print-after-all and I get this ... !2 = distinct !{!2, !3} !3 = !{!"llvm.loop.unroll.count", i32 3} !4 = !{!5, !5, i64 0} !5 = !{!"int", !6, i64 0} !6 = !{!

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

bjope added a comment. In https://reviews.llvm.org/D48721#1150361, @hfinkel wrote: > In https://reviews.llvm.org/D48721#1150333, @bjope wrote: > > > Is the fault that the metadata only should be put on the back edge, not the > > branch in the preheader? > > > Yea. Our past thinking has been that

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

bjope added inline comments. Comment at: test/CodeGen/pragma-do-while.cpp:1 +// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s +int test(int a[], int n) { I think that we can simplify it to use one loop here (as a regression test that we only put the label on

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

bjope added a comment. In https://reviews.llvm.org/D48721#1153138, @deepak2427 wrote: > I have updated the test to not run the optimizer. The test I had added > previously for checking if the unroller is respecting the pragma is useful I > think. Not sure where that can be added though. > I g

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

bjope accepted this revision. bjope added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

bjope added a comment. In https://reviews.llvm.org/D48721#1157571, @deepak2427 wrote: > @Bjorn, Thanks for reviewing and accepting the patch. > > Could you please advise on the next steps? > Would someone else commit this on my behalf or should I request for commit > access? > > Thanks, > Deep

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

This revision was automatically updated to reflect the committed changes. Closed by commit rL336717: Patch to fix pragma metadata for do-while loops (authored by bjope, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D48721?vs=154244&id

[PATCH] D69261: Prune Pass.h include from DataLayout.h. NFCI

bjope created this revision. bjope added a reviewer: rnk. Herald added subscribers: cfe-commits, hiraditya, mehdi_amini. Herald added projects: clang, LLVM. Reduce include dependencies by no longer including Pass.h from DataLayout.h. That include seemed irrelevant to DataLayout, as wee as lots of

[PATCH] D69262: Prune include of DataLayout.h from include/clang/Basic/TargetInfo.h. NFC

bjope created this revision. bjope added a reviewer: rnk. Herald added subscribers: cfe-commits, nhaehnle, jvesely. Herald added a project: clang. Use a forward declaration of DataLayout instead of including DataLayout.h in clangs TargetInfo.h. This reduces include dependencies toward DataLayout.h

[PATCH] D69261: Prune Pass.h include from DataLayout.h. NFCI

This revision was automatically updated to reflect the committed changes. Closed by commit rG1f43ea41c330: Prune Pass.h include from DataLayout.h. NFCI (authored by bjope). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69261/new/ https://reviews.llv

[PATCH] D69262: Prune include of DataLayout.h from include/clang/Basic/TargetInfo.h. NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rG78424e5f8417: Prune include of DataLayout.h from include/clang/Basic/TargetInfo.h. NFC (authored by bjope). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D692

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

bjope added inline comments. Comment at: clang/test/Driver/wasm-toolchain.c:116 +// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" + -

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

bjope added inline comments. Comment at: clang/test/Driver/wasm-toolchain.c:116 +// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" + -

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

bjope added inline comments. Comment at: clang/test/Driver/wasm-toolchain.c:116 +// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" + -

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

bjope added a comment. I think the warnings can be hidden by something like this (I'm no expert in this area though, but it seems like this technique has been used a number of times before in llvm, so hopefully it applies here as well): diff --git a/clang-tools-extra/clangd/Preamble.cpp b/cl

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

bjope added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:2660 return; + if (SrcType->isIntegerType() && DestType->isFixedPointType()) +return; Is this really the intention with the patch? It does match the "summary" above, but isn't the war

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

bjope added inline comments. Comment at: clang/test/Sema/warn-bad-function-cast.c:49 +#ifdef FIXED_POINT + (void)(_Fract) if1(); // no warning +#endif bjope wrote: > bjope wrote: > > This should be added before the line saying `/* All following casts issue > >

[PATCH] D85157: [Sema] Add casting check for integer to fixed point conversions

bjope added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:2660 return; + if (SrcType->isIntegerType() && DestType->isFixedPointType()) +return; vabridgers wrote: > bjope wrote: > > Is this really the intention with the patch? > > > > It does

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

bjope added a comment. In D85191#2196863 , @rsmith wrote: > In D85191#2195923 , @ebevhan wrote: > >> In D85191#2193645 , @rsmith wrote: >> This is not ideal, since it ma

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

bjope added inline comments. Comment at: clang/test/Sema/warn-bad-function-cast.c:49 +#ifdef FIXED_POINT + (void)(_Fract) if1(); // no warning +#endif bjope wrote: > vabridgers wrote: > > bjope wrote: > > > bjope wrote: > > > > bjope wrote: > > > > > This should

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

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

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

bjope added a comment. In D80525#2215641 , @adamcz wrote: > This is quite strange. I can't reproduce this with gcc 7.5.0, 9.3.0 or > 10.0.1. Which version are you using? > > There are bugs around clangd + modules + diagnostics. I'm fixing one in > https:

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

bjope added a comment. Might be that it is some other commit that cause the problem. Looking a bit closer at the jenkins history it seems like the tests passed once after this patch was added. But the next time it failed. That second time the "[Diagnoostics] Reworked -Wstring-concatenation" com

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

bjope added a comment. In D80525#2215778 , @adamcz wrote: > Looks like it's an issue with not clearing the module cache. I'll revert this > change to fix the build, then re-submit with proper fix. Ok, thanks. Let me know if you want me to investigate fur

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

bjope added inline comments. Comment at: clang/test/Sema/constant-conversion.c:30 + s.b = 1; // one-bit-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}} + s.b = true; // no-warning (we suppress it manually to reduce false

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

bjope added inline comments. Comment at: clang/test/Sema/constant-conversion.c:30 + s.b = 1; // one-bit-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}} + s.b = true; // no-warning (we suppress it manually to reduce false

[PATCH] D128472: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens.

bjope added a comment. Our downstream bots have failed when using `LLVM_ENABLE_EXPENSIVE_CHECKS=ON` , and I think (unless my bisecting got screwed up) that it started to happen after this patch. We typically get Failed Tests (10): ClangPseudo :: lr-build-basic.test ClangPseudo :: lr-

[PATCH] D128472: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens.

bjope added a comment. In D128472#3625584 , @hokein wrote: > Thanks for the report. There is an out-of-bound issue in > LRTable::getReduceRules, fixed in c99827349927a44334f2b04139168efd0bc87cd3 >

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

bjope added a comment. Have had problems with failing stage 2 builds since this patch: FAILED: tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o /repo//install/stage2/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MAC

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

bjope added a comment. In D121233#3426155 , @sammccall wrote: > In D121233#3425793 , @bjope wrote: > >> Have had problems with failing stage 2 builds since this patch: > > Should be fixed in 72ae6cc3a689869dcc15cf

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

bjope added a comment. Hello. We've got some problem in our downstream tests after this patch and I'm trying to figure out how things are supposed to work. Maybe someone being part of this review knows. Problem is that we have some libc verification suites that include test cases using `float_

[PATCH] D121122: Set FLT_EVAL_METHOD to -1 when fast-math is enabled.

bjope added a comment. Looked around a bit. - Seems like in GNU libc they use an internal `__GLIBC_FLT_EVAL_METHOD` that is set to 2 if `__FLT_EVAL_METHOD__` is -1 (https://sourceware.org/git/?p=glibc.git;a=blob;f=bits/flt-eval-method.h;hb=f60e45ba10f0ca2794318de95720cdbdb6ff20d0). - In LLVM:

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

bjope added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:581 RTLIB::POWI_F128, RTLIB::POWI_PPCF128); if (!TLI.getLibcallName(LC)) { efriedma wrote: > T

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

bjope added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:581 RTLIB::POWI_F128, RTLIB::POWI_PPCF128); if (!TLI.getLibcallName(LC)) { efriedma wrote: > b

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

bjope created this revision. bjope added reviewers: yaxunl, tra, zhuhan0. bjope requested review of this revision. Herald added a project: clang. The checks (both positive and negative checks) in the test case hip-include-path.hip could mistakenly end up matching the string "clang" from the Instal

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

This revision was automatically updated to reflect the committed changes. Closed by commit rG722c39fef5ab: [HIP] Tighten checks in hip-include-path.hip test case (authored by bjope). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102723/new/ https://

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

bjope marked 3 inline comments as done. bjope added inline comments. Comment at: llvm/test/Transforms/InstCombine/pow_fp_int16.ll:3 -; PR42190 +; Test case was copied from pow_fp_int.ll but adjusted for 16-bit int. +; Assuming that we can't generate test checks for the same rea

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

bjope added a comment. In D102723#2789537 , @yaxunl wrote: > It seems we cannot introduce `ROOT` by line 19 since it is not used in other > lines in situations where working directories have sym links. > > Instead, we just change `{{.*}}clang/` to `{{.*}

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

bjope added a comment. I relaxed the checks a bit again here: https://reviews.llvm.org/rGf0e10cc91bc4 But it looks like the workers here (https://lab.llvm.org/staging/#/workers/109) are paused so hard to tell if it helped. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D103386: [PowerPC] Fix x86 vector intrinsics wrapper compilation under C++

bjope added subscribers: phosek, bjope. bjope added inline comments. Comment at: clang/test/CodeGen/ppc-xmmintrin.c:10 // RUN: -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE +// RUN: %clang -x c++ -fs

[PATCH] D101192: Add support for #elifdef and #elifndef

bjope added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:594 +if (CondInfo.FoundElse) + Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif; Hi @aaron.ballman This change is missing from https://reviews.llvm.org/rG8edd3464afbf

[PATCH] D101192: Add support for #elifdef and #elifndef

bjope added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:594 +if (CondInfo.FoundElse) + Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif; aaron.ballman wrote: > bjope wrote: > > Hi @aaron.ballman > > > > This change is missi

[PATCH] D103386: [PowerPC] Fix x86 vector intrinsics wrapper compilation under C++

bjope added inline comments. Comment at: clang/test/CodeGen/ppc-xmmintrin.c:10 // RUN: -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE +// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-l

[PATCH] D94977: [CodeGen] Use getCharWidth() more consistently in CGRecordLowering. NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG72f863fd37c3: [CodeGen] Use getCharWidth() more consistently in CGRecordLowering. NFC (authored by bjope). Repository: rG LLVM Github Monorepo CH

[PATCH] D94979: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGea2cfda386f1: [CGExpr] Use getCharWidth() more consistently in CCGExprConstant. NFC (authored by bjope). Changed prior to commit: https://reviews.

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

bjope added a comment. Kind of the same take on this as @thopre . I wrote https://bugs.llvm.org/show_bug.cgi?id=48821 , and proposed https://reviews.llvm.org/D95789 , when I noticed that this pass caused troubles for two reasons: 1. to avoid that others would stumble upon the same problem (get

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

bjope added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:1449 - // inserting redundancies into the program. This even includes SimplifyCFG. - OptimizePM.addPass(SpeculateAroundPHIsPass()); - lebedev.ri wrote: > nikic wrote: > > As it has been in

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

bjope added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99439/new/ https://reviews.llvm.org/D99439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

bjope added a comment. In D99439#2823338 , @efriedma wrote: > LGTM Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99439/new/ https://reviews.llvm.org/D99439

[PATCH] D105007: [NewPM] Update some sanitizer pass names in the PassRegistry

bjope updated this revision to Diff 367263. bjope added a comment. Herald added subscribers: cfe-commits, ormris. Herald added a project: clang. Updated to use _PASS_WITH_PARAMS and option parsing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D10500

[PATCH] D105007: [NewPM] Make some sanitizer passes parameterized in the PassRegistry

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG36d51386195e: [NewPM] Make some sanitizer passes parameterized in the PassRegistry (authored by bjope). Changed prior to commit: https://reviews.l

[PATCH] D105974: [analyzer] Do not assume that all pointers have the same bitwidth as void*

bjope added a comment. I think you can create an upstream test case by looking at `clang/test/Analysis/ptr.cl`. It uses amdgcn and address space 256, which seems to have a different pointer size compared to address space zero for that specific target. So the test case (named with a .cl extensio

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

bjope added a comment. This seem to cause some weird results. Given this input: bar(short k) { k++; for (short f = 0; f < k; f++) ; (long)k << 16; } we get > clang --analyze --target=x86_64 'bbi-63538.c' bbi-63538.c:5:11: warning: The result of the left shift is unde

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

bjope added a comment. FWIW, just some things noticed when I examined some of the new warning that popped up after this patch: https://github.com/llvm/llvm-project/issues/53253 mentioned that for example gcc complained about this. Although, as shown here https://godbolt.org/z/bq34Kexac there a

[PATCH] D153092: [Clang][CodeGen]`vtable`, `typeinfo` et al. are globals

bjope added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:693 return Int32Ty; - return Int8PtrTy; + return GlobalsInt8PtrTy; } I noticed that we have some old fixes downstream that conflicts with the changes you've made here. I thought tha

[PATCH] D156733: Stop using legacy helpers indicating typed pointer types. NFC

bjope created this revision. Herald added subscribers: nlopes, Enna1, pmatos, asb, jeroen.dobbelaere, pengfei, hiraditya, jgravelle-google, sbc100, dschuff. Herald added a project: All. bjope requested review of this revision. Herald added subscribers: cfe-commits, wangpc, aheejin. Herald added pr

[PATCH] D156733: Stop using legacy helpers indicating typed pointer types. NFC

bjope updated this revision to Diff 545783. bjope added a comment. Herald added a subscriber: ormris. Replaced a couple of more occurances in this update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156733/new/ https://reviews.llvm.org/D156733 F

[PATCH] D156733: Stop using legacy helpers indicating typed pointer types. NFC

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfd05c34b18fb: Stop using legacy helpers indicating typed pointer t

[PATCH] D156911: [clang][CodeGen] Drop some typed pointer bitcasts

bjope created this revision. Herald added a subscriber: nlopes. Herald added a project: All. bjope requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D156911 Files: clang/lib/C

[PATCH] D156911: [clang][CodeGen] Drop some typed pointer bitcasts

bjope updated this revision to Diff 546921. bjope added a comment. Rebased+updated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156911/new/ https://reviews.llvm.org/D156911 Files: clang/lib/CodeGen/CGBlocks.cpp clang/lib/CodeGen/CGBuiltin.cp

[PATCH] D156911: [clang][CodeGen] Drop some typed pointer bitcasts

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2bdc86484d03: [clang][CodeGen] Drop some typed pointer bitcasts (a

[PATCH] D157550: [clang] Drop some references to typed pointers (getInt8PtrTy). NFC

bjope created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. bjope requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D157550 Files: clang/li

[PATCH] D157550: [clang] Drop some references to typed pointers (getInt8PtrTy). NFC

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGd03f4177dfbb: [clang] Drop some references to typed pointers (getInt8PtrTy). NFC (authored by bjope). Repository: rG LL

[PATCH] D76533: [clangd] Skip ClangdVFSTest.TestStackOverflow when address sanitizer is used

bjope created this revision. bjope added reviewers: sammccall, Dmitry.Kozhevnikov. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. bjope added a comment. bjope added subscribers: uabelho, dstenb. I don't know if

[PATCH] D76533: [clangd] Skip ClangdVFSTest.TestStackOverflow when address sanitizer is used

bjope added a comment. I don't know if this is a good solution. But we've had problems with our downstream bots that builds llvm with asan, and runs check-all, for awhile (probably since we merged https://reviews.llvm.org/D50993). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D76533: [clangd] Skip ClangdVFSTest.TestStackOverflow when address sanitizer is used

This revision was automatically updated to reflect the committed changes. Closed by commit rGc6d799156a0a: [clangd] Skip ClangdVFSTest.TestStackOverflow when address sanitizer is used (authored by bjope). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D70203: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

bjope added a comment. Hi @sammccall . Just a heads up. Looks like this might have caused: https://bugs.llvm.org/show_bug.cgi?id=44143 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70203/new/ https://reviews.llvm.org/D70203 _

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

bjope added a comment. When I build the clang on trunk, using clang 8.0 with asan enabled, and then run the clang lit tests I see lots of failures in Clang :: InterfaceStubs/driver-test.c and Clang :: InterfaceStubs/driver-test2.c (and maybe the faults I get in Clang :: Driver/cl-showfilenames.

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

bjope added a comment. I get errors when compiling due to this commit: FAILED: tools/clang/lib/Tooling/CMakeFiles/clangTooling.dir/StandaloneExecution.cpp.o /repo/app/clang/3.6/bin/clang++ -march=corei7 -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MA

  1   2   >