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

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

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] 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] D145868: [clang][ASTImporter] Fix import of typedef with unnamed structures

bjope added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:8627 + )"; + Decl *ToTU = getToTuDecl("", Lang_CXX11); + Decl *FromTU = getTuDecl(Code, Lang_CXX11); balazske wrote: > bjope wrote: > > With Werror we get: > > > > ``` > > ..

[PATCH] D145868: [clang][ASTImporter] Fix import of typedef with unnamed structures

bjope added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:8627 + )"; + Decl *ToTU = getToTuDecl("", Lang_CXX11); + Decl *FromTU = getTuDecl(Code, Lang_CXX11); With Werror we get: ``` ../../clang/unittests/AST/ASTImporterTest.cpp:862

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

bjope added a comment. In D126959#4222854 , @iains wrote: > In D126959#4222637 , @bjope wrote: > >> This seem to case problems when building with asan enabled >> (LLVM_USE_SANITIZER='Address'): > > investigating.

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

bjope added a comment. This seem to case problems when building with asan enabled (LLVM_USE_SANITIZER='Address'): Failed Tests (24): Clang :: CXX/basic/basic.link/p2.cpp Clang :: CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp Clang :: CXX/basic/basic.scope/basic.scope.namesp

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

bjope added a comment. I have no objections about doing this revert. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145704/new/ https://reviews.llvm.org/D145704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[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] 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] 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] 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] 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] 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] 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] 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] 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] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

bjope added a comment. I actually see lots of these as well: clang: ../lib/Support/APInt.cpp:284: int llvm::APInt::compareSigned(const llvm::APInt &) const: Assertion `BitWidth == RHS.BitWidth && "Bit widths must be same for comparison"' failed. Although I haven't verified if those are relat

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

bjope added a comment. I get failures after having merged this patch: /llvm/build-all/bin/clang -Xanalyzer -analyzer-output=text -Xanalyzer -analyzer-checker='nullability,optin.portability.UnixAPI,security,valist' -Xanalyzer -analyzer-disable-checker='security.insecureAPI.strcpy' --analyze c

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

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] 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] 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] 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] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

bjope added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:765 +R = StateMgr.getStoreManager().castRegion(ER, CastTy); +return loc::MemRegionVal(R); + } This caused some problems with assertion failures, see https

[PATCH] D101194: [Driver] Push multiarch path setup to individual drivers

bjope added a comment. In D101194#2720787 , @phosek wrote: > In D101194#2720761 , @bjope wrote: > >> Or maybe the problem is that we use `-DLLVM_BUILTIN_TARGETS=default`? > > The use of `default` is the issue, the

[PATCH] D101194: [Driver] Push multiarch path setup to individual drivers

bjope added a comment. Got some problems with my builds after this commit. My cmake commands looks like this (well, this is after having tried to fix the problem by replacing `x86_64-unknown-linux-gnu` by `x86_64-linux-gnu` as I figured that might be needed after looking at the changes in this

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

bjope added inline comments. Comment at: llvm/test/Transforms/InstCombine/pow-4.ll:4 +; RUN: opt -instcombine -S < %s -mtriple unknown -disable-builtin sqrt | FileCheck %s --check-prefixes=CHECK,CHECKI32,NOSQRT +; RUN: opt -instcombine -S < %s -mtriple msp430

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

bjope added inline comments. Comment at: llvm/test/Transforms/InstCombine/pow_fp_int16.ll:1 -; NOTE: Assertions have been autogenerated by utils/update_test_checks.py -; RUN: opt -mtriple unknown -instcombine -S < %s | FileCheck %s +; RUN: opt -mtriple msp430 -instcombine -S < %s

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

bjope added inline comments. Comment at: llvm/docs/LangRef.rst:13280 +floating-point or vector of floating-point type. The type of the exponent +should typically match the size of ``int``, at least when the intrinsic maps +to one of the ``__powi*`` functions in compiler-rt. Not a

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

bjope added a comment. Ping! (This patch is now 1 month old, has a number of reviewers and 41 subscribers, but not a single comment yet. I believe that if you aren't comfortable with reviewing, then it is perfectly OK to remove yourself as reveiwer to let the author know that the original set

[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] D81285: [builtins] Change si_int to int in some helper declarations

bjope added a comment. Hi! My users have found problems with miscompiles that seem to originate from this patch. As a background, my target has 16-bit int, and 32-bit long. And the calling convention is not that an i16 is passed as the low bits in a 32-bit register that would be used for an i

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

bjope added a comment. @lebedev.ri : I think I need your blessing as well on this, considering your earlier concerns. Is it still just confusing? (I doubt that we want/can replace all uses of getCharWidth/getIntWidth etc in clang with integers) Repository: rG LLVM Github Monorepo CHANGES SI

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

bjope updated this revision to Diff 318135. bjope added a comment. Updated to add CharTy to the CodeGenTypeCache (based on review feedback). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94979/new/ https://reviews.llvm.org/D94979 Files: clang/li

[PATCH] D94979: [CGExpr] Honor getCharWidth() in ConstantAggregateBuilderUtils

bjope added a comment. In D94979#2507242 , @lebedev.ri wrote: > As far as i recall, all RFC's about LLVM support for non-8-bit char targets > didn't end in favor of the proposal > I'd say that until there's general consensus/buy-in (including testing path

[PATCH] D94979: [CGExpr] Honor getCharWidth() in ConstantAggregateBuilderUtils

bjope created this revision. bjope requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In CGExprConstant.cpp, when dealing with padding etc, some sizes are calculated as char units. But then the type used when creating the aggregate expression

[PATCH] D94977: [CodeGen] Honor getCharWidth() in CGRecordLowering

bjope created this revision. bjope requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When using getByteArrayType the requested size was calculated in char units, but the type used for the array was hardcoded to the Int8Ty. Honor the size of c

[PATCH] D86632: [Fixed Point] Add codegen for conversion between fixed-point and floating point.

bjope added a comment. I've just landed this on behalf of @ebevhan. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86632/new/ https://reviews.llvm.org/D86632 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D86632: [Fixed Point] Add codegen for conversion between fixed-point and floating point.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4944a6f53f6: [Fixed Point] Add codegen for conversion between fixed-point and floating point. (authored by ebevhan, committed by bjope). Herald add

[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] 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#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] 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] 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] 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 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] 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; Is this really the intention with the patch? It does match the "summary" above, but isn't the war

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

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

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

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

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()); + This has caused problem for our sanitizer tests the last couple of day

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

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

  1   2   >