[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem created this revision. Herald added subscribers: ellis, abrachet, phosek, hiraditya. Herald added a project: All. gulfem requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. CoverageMappingModuleGen generates a coverage

[PATCH] D121812: [clang][deps] NFC: De-duplicate clang-cl tests

gulfem added a comment. Newly added tests `ClangScanDeps/cl-output.c` and `ClangScanDeps/cl-xclang.c` started failing in our Mac toolchain builds: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b881168677577537/overview Here's the link to the log: https://logs.

[PATCH] D121812: [clang][deps] NFC: De-duplicate clang-cl tests

gulfem added a comment. In D121812#3405060 , @jansvoboda11 wrote: > Sorry for the breakage and thanks for reporting this! This is a real bug > uncovered by your build using `/opt`. I have a fix here: D122385 > . Thank

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem updated this revision to Diff 417945. gulfem added a comment. Addressed phosek's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122336/new/ https://reviews.llvm.org/D122336 Files: clang/test/CoverageMapping/unused_function_no_runt

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. In D122336#3405835 , @davidxl wrote: > Should this be fixed in Clang (not generating coverage record)? Adding the > logic here is a little confusing. @davidxl, I originally did that (not generating coverage records) because I thi

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. In D122336#3406186 , @vsk wrote: > Sorry for ay delayed replies - I've switched teams at Apple and find it > difficult to keep up with llvm reviews. > >> it's my understanding is that we might be generating coverage record for >>

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. In D122336#3406843 , @vsk wrote: > So long as it doesn't change behavior expected by tapi on Darwin, I think > it's OK. I doubt any other platforms are similarly affected. I don't think it should impact TAPI because it relies on

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc7f91e227a79: [InstrProfiling] No runtime hook for unused funcs (authored by gulfem). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:569 + if (!containsProfilingIntrinsics(M)) { +if (!CoverageNamesVar || !NeedsRuntimeHook) { + return MadeChange; MaskRay wrote: > https://llvm.org/docs/Codi

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. @davidxl, I added comments in https://github.com/llvm/llvm-project/commit/ead8586645f54de16cfc6fa26028d818efc425f2 @MaskRay, I followed the style guide for nested if statements in https://github.com/llvm/llvm-project/commit/ead8586645f54de16cfc6fa26028d818efc425f2 Repo

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#3712880 , @nlopes wrote: > In D90275#3707793 , @nlopes wrote: > >> In D90275#3671983 , @gulfem wrote: >> >>> In D90275#3671019

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#3712991 , @nlopes wrote: > In D90275#3712969 , @gulfem wrote: > >> In D90275#3712880 , @nlopes wrote: >> >>> In D90275#3707793

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. Added the description in https://reviews.llvm.org/D131628 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 ___ cfe-commits mailing list cfe

[PATCH] D118862: [clang][driver] add clang driver support for emitting macho files with two build version load commands

gulfem added a comment. The following two tests started failing in our bots: Clang :: Driver/darwin-ld-platform-version-target-version.c Clang :: Driver/darwin-zippered-target-version.c https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8822219707275734081/over

[PATCH] D119544: Deferred Concept Instantiation Implementation

gulfem added a comment. We started seeing several test failures after this commit: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8815265760499763361/overview One example is `nothrow_forward_range.compile.pass.cpp`. Script: -- : 'COMPILED WITH'; /b/s/w/i

[PATCH] D119544: Deferred Concept Instantiation Implementation

gulfem added a comment. In D119544#3486241 , @erichkeane wrote: > Ah shucks... Thanks for the heads up. Is there any chance to get you to get > me a pre-processed version of this failure to play with? I've not had luck > compiling/running libc++ test

[PATCH] D119544: Deferred Concept Instantiation Implementation

gulfem added a comment. > Then pipe that to a file (note the -E I added at the end). You should get a > file that looks like some slightly-wonky C++ code. I got the following output after running it via `-E`. /usr/local/google/home/gulfem/llvm-clean/llvm-project/libcxx/test/libcxx/algorithm

[PATCH] D126586: [InstrProf][WIP] Implement boolean counters in coverage

gulfem added a comment. @davidxl and @vsk, This is a WIP prototype implementation, and I would love to hear your early feedback on using 1-byte counters for coverage idea if possible. I'll continue extending the prototype, but that would be great to learn if there is anything fundamentally wrong

[PATCH] D126586: [InstrProf][WIP] Implement boolean counters in coverage

gulfem added a comment. In D126586#3646197 , @davidxl wrote: > For coverage mode, why using 'incrementProfileCounter'? Should it be set to > '1' instead? Also is the 'or' expression needed? The expression can be > folded to either zero or 1. @davidxl

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#3671019 , @nlopes wrote: > Could you please add a description of this attribute to LangRef? > We need the semantics of this. > > Thank you! Sure, I'll do that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 310269. gulfem added a comment. Added more Sema test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td clang/include/cla

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/test/Sema/attr-leaf.c:4 + +void f() __attribute__((leaf)); aaron.ballman wrote: > Can you also add tests that show the attribute diagnoses being written on the > wrong subje

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 310290. gulfem marked an inline comment as done. gulfem added a comment. Add a new line at the end of the test file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. > This is missing a lang ref entry for `nocallback` and the `attributes.ll` > test is arguably broken (see below). Could you please elaborate on missing lang ref entry? Where that should be added? > The "definition" in `llvm/include/llvm/IR/Attributes.td` (see below), d

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. > The problem in `llvm/test/Bitcode/attributes.ll` is clear? Instead of `define void @f70() nocallback`, it should be `define void @f69() nocallback`, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https:/

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. > I fixed it in > https://reviews.llvm.org/D92493 > But that was just to make my test pass. Ok, thank you! I did not realize that typo after the merge, and sorry about that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D902

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added a comment. In D90275#2371343 , @jdoerfert wrote: > The more I think about it, the more I think we should never create a > `leaf`/`nocallback` definition. Only declarations should carry that attribute.

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 302716. gulfem added a comment. Add a target into the test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td clang/includ

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3910 +in library functions. Functions marked with the ``leaf`` attribute are not allowed +to jump back into the caller's translation unit, whether through invoking a +callback function, a direct exte

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#2371343 , @jdoerfert wrote: > The more I think about it, the more I think we should never create a > `leaf`/`nocallback` definition. Only declarations should carry that attribute. Leaf attribute is specifically intended

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 303200. gulfem added a comment. Remove direct from description and ObjCMethod for Obj functionality Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/includ

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gu

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gu

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > > > aaron

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as not done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > gulfe

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as not done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote:

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 310052. gulfem added a comment. Only support leaf attribute in functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td cla

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jdoerfert, steven_wu, hiraditya. Herald added a reviewer: aaron.ballman. Herald added projects: clang, LLVM. gulfem requested review of this revision. GCC supports __attribute__((leaf)) as an optimizati

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#2357945 , @jdoerfert wrote: > In D90275#2357931 , @mcgrathr wrote: > >> There is an RFC going out with this prototype as reference. When there is >> consensus on the RFC, this wil

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 301787. gulfem added a comment. Add IR and bitcode tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td clang/include/clang

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 301789. gulfem added a comment. Fix the typo in the test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td clang/include/

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 302349. gulfem marked 3 inline comments as done. gulfem added a comment. Addressed the comments about the documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 F

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as not done. gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3909 +The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis in library functions. +Functions marked as ``leaf`` attribute are not

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > Should this attribute also be supported on

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 302369. gulfem added a comment. Update the attribute documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td clang/inc

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3910 +in library functions. Functions marked with the ``leaf`` attribute are not allowed +to jump back into the caller's translation unit, whether through invoking a +callback function, a direct exte

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3909 +The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis in library functions. +Functions marked as ``leaf`` attribute are not all

[PATCH] D102286: [HWASan] Build separate LAM runtime on x86_64.

gulfem added a comment. We started seeing test failures in our Windows builds in Fuchsia after this patch. FAILED: compiler-rt/lib/hwasan/CMakeFiles/RTHwasanAliases.aarch64.dir/hwasan_interceptors.cpp.o C:\b\s\w\ir\x\w\staging\llvm_build\.\bin\clang++.exe --target=aarch64-unknown-linux-g

[PATCH] D102286: [HWASan] Build separate LAM runtime on x86_64.

gulfem added a comment. > Looks like the same issue I fixed in https://reviews.llvm.org/rGd97bab651185. > Let me know if you have the issue after that commit. Thanks, that resolved the build issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D10

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

gulfem updated this revision to Diff 318372. gulfem added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. 1. Apply relative lookup table generation in fPIC mode 2. Add a test case for single value case 3. Remove hard-coding 64 bit pointers 4. Improve implementatio

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

gulfem updated this revision to Diff 320025. gulfem added a comment. 1. Simplified test cases and increased readibility of the tables 2. Added x86_64 and aarch64 check and tiny or small code modes check to ensure 32 offsets 3. Modified single value test case Repository: rG LLVM Github Monorep

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

gulfem marked 9 inline comments as done. gulfem added inline comments. Comment at: clang/test/CodeGen/switch-to-lookup-table.c:2 +// Check switch to lookup optimization in fPIC and fno-PIC mode +// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O2 -fno-PIC -S -emit-llvm -o

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

gulfem marked an inline comment as done. gulfem added a comment. In D94355#2530225 , @lebedev.ri wrote: > Can you please add an explanation to the patch's description as to why > we don't want to instead convert non-relative/relative LUT's elsewhere, > ple

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

gulfem added a comment. We also started seeing similar `libc++` test failures in our Fuchsia builds: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8835548361443044001/+/u/clang/test/stdout Failed Tests (89): libc++ :: std/numerics/rand/rand.adapt/rand.adapt.ibits/assign.

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

gulfem updated this revision to Diff 324144. gulfem marked an inline comment as not done. gulfem added a comment. Herald added a subscriber: mgorny. Implement it as a separate pass and apply it to user-defined lookup tables as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

gulfem added a comment. @lebedev.ri based on your feedback, I added it as a separate pass and added support for user-defined lookup tables. Please let me know if you have any comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94355/new/ https

[PATCH] D94355: [Passes] Add relative lookup table generator pass

gulfem updated this revision to Diff 326265. gulfem added a comment. Herald added subscribers: wenlei, nikic, kerbowa, steven_wu, nhaehnle, jvesely. - Rename the pass to RelLookupTableConverter to be consistent - Addressed reviewers' feedback - Added tests for user-defined lookup tables and hidden

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem marked 17 inline comments as done. gulfem added inline comments. Comment at: clang/test/CodeGen/switch-to-lookup-table.c:2 +// Check switch to lookup optimization in fPIC and fno-PIC mode +// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O2 -fno-PIC -S -emit-llvm -o

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem added a comment. > Thanks for pushing this forward! I think this will be a nice transformation > once all the details are worked out. Thank you very much for all of your wonderful constructive feedback! I learned much more about LLVM and IR internals. Appreciate all your help! Reposito

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem added a comment. In D94355#2572553 , @leonardchan wrote: > It looks like you have everything setup for running on the new PM, but it > doesn't look like the pass is added anywhere in the new PM pipeline. Thank you very much for pointing that out @

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem added inline comments. Comment at: llvm/lib/Transforms/Utils/RelLookupTableGenerator.cpp:76-77 + + /// If lookup table has more than one user, + /// do not generate a relative lookup table. + if (!GlobalVar.hasOneUser()) leonardchan wrote: > What's the

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem added inline comments. Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:73 + return false; + +// If an operand in the lookup table is not dso_local, pcc wrote: > In the version of the patch that you committed, you have this check

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem closed this revision. gulfem added a comment. Since this patch has been merged and the issue in the test has been resolved, I'm closing this review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 _

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem updated this revision to Diff 328354. gulfem added a comment. Use TTI hook for target machine checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94355/new/ https://reviews.llvm.org/D94355 Files: clang/test/CodeGen/switch-to-lookup-table

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem updated this revision to Diff 330405. gulfem added a comment. 1. Used IsConstantOffsetFromGlobal() function 2. Added this pass to the end of legacy pass manager pipeline 3. Moved all the tests under a new test directory Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D94355: [Passes] Add relative lookup table converter pass

gulfem marked 7 inline comments as done. gulfem added inline comments. Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:384 + bool shouldBuildRelLookupTables() { +const TargetMachine &TM = getTLI()->getTargetMachine(); leonardchan wrote: > Sorry, I thi

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

gulfem added a comment. Can somebody please clarify this? Is the following code results `undefined` behavior? enum E1 {e11=-4, e12=4}; E1 x2b = static_cast(8); `constexpr E1 x2 = static_cast(8)` seems like `undefined`, so `-Wenum-constexpr-conversion` is triggered. We started seeing `-Wenu

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

gulfem added a comment. In D131528#3717509 , @shafik wrote: > In D131528#3717141 , @gulfem wrote: > >> Can somebody please clarify this? Is the following code results in >> `undefined` behavior? >> >> enum E1 {

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

gulfem added a comment. We are seeing the same error reported above: FormatTests: clang/lib/Lex/Lexer.cpp:1151: SourceLocation clang::Lexer::getSourceLocation(const char *, unsigned int) const: Assertion `Loc >= BufferStart && Loc <= BufferEnd && "Location out of range for this buffer!"' fai

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

gulfem added a comment. In D135356#3840428 , @sammccall wrote: > Fixed by 4b53c00173163774d32125fbcae283a46a9a4b19 > I think. It fixed the test error that we are seeing, thanks! Rep

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

gulfem added a comment. We also started seeing `-Wthread-safety-precise` error in our Fuchsia code. https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-release/b8800959115965408001/overview I'm trying to verify with our team whether it is a false positive, but I jus

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

gulfem added a comment. In D129755#3843206 , @aaronpuchert wrote: > In D129755#3843144 , @gulfem wrote: > >> We also started seeing `-Wthread-safety-precise` error in our Fuchsia code. >> https://luci-milo.appsp

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

gulfem added a comment. In D129755#3844059 , @aaronpuchert wrote: > In D129755#3843144 , @gulfem wrote: > >> We also started seeing `-Wthread-safety-precise` error in our Fuchsia code. >> https://luci-milo.appsp

[PATCH] D143304: [Coverage] Map regions from system headers

gulfem created this revision. Herald added a project: All. gulfem requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Originally, the following commit removed mapping coverage regions for system headers: https://github.com/llvm/llvm-project/co

[PATCH] D143304: [Coverage] Map regions from system headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG2155195131a5: [Coverage] Map regions from system headers (authored by gulfem). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143304/new/ https://reviews.llv

[PATCH] D127040: [InstrProf][WIP] Implement boolean counters in coverage

gulfem created this revision. Herald added subscribers: ellis, wenlei, hiraditya. Herald added a project: All. gulfem requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This patch inserts 1-byte boolean counters instead of an

[PATCH] D126586: [InstrProf][WIP] Implement boolean counters in coverage

gulfem updated this revision to Diff 434224. gulfem added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Describe the high-level changes to adding and subtracting counters. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D157944: [InstrProf] Single byte counters in coverage

gulfem created this revision. Herald added subscribers: wlei, ellis, wenlei. Herald added a project: All. gulfem requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch inserts 1-byte counters instead of an 8-byte counters into llvm prof

[PATCH] D126586: [InstrProf][WIP] Implement boolean counters in coverage

gulfem updated this revision to Diff 550163. gulfem added a comment. - Extend the prototype by implementing the rest of the control-flow statements - Rename the prototype to single byte counters to be consistent with the single byte coverage used in PGO Repository: rG LLVM Github Monorepo CH

[PATCH] D126586: [InstrProf] Single byte counters in coverage

gulfem updated this revision to Diff 551686. gulfem added a comment. Ensure the correct traversal order for loops in CodeGenPGO to fix the profile test failures (Profile/cxx-hash-v2.cpp and Profile/cxx-rangefor.cpp) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D126586: [InstrProf] Single byte counters in coverage

gulfem updated this revision to Diff 551687. gulfem added a comment. Remove hyphen between "single counter" for consistency Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126586/new/ https://reviews.llvm.org/D126586 Files: clang/lib/CodeGen/CGExp

[PATCH] D126586: [InstrProf] Single byte counters in coverage

gulfem updated this revision to Diff 553992. gulfem added a comment. Fixed the issue that causes a crash in InstCombinePHI.cpp. Phi instructions need to be inserted at the beginning of basic blocks, and profile increments need to inserted after phis for conditional operators. Repository: rG LL

[PATCH] D126586: [InstrProf] Single byte counters in coverage

gulfem updated this revision to Diff 42. gulfem added a comment. 1. Use || to merge counters that refer to the same regions 2. Use || to merge counters from raw profiles Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126586/new/ https://reviews.

[PATCH] D126586: [InstrProf] Single byte counters in coverage

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:1080-1081 +void CodeGenPGO::setProfileVersion(llvm::Module &M) { + if (CGM.getCodeGenOpts().hasProfileClangInstr() && + llvm::EnableSingleByteCoverage) { +

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

gulfem added a comment. We started seeing a test failure in `avr-ld.c`. /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c:48:11: error: // LINKP: {{".*ld.*"}} {{.*}} "--defsym=__DATA_REGION_ORIGIN__=0x800100" "-plugin" {{.*}} "-plugin-opt=mcpu=atmega328" ^ :1:1: note:

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

gulfem added a comment. In D145646#4213975 , @MaskRay wrote: > In D145646#4213547 , @gulfem wrote: > >> We started seeing a test failure in `avr-ld.c`. >> >> /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr

[PATCH] D148757: [clang] Follow user-provided order for prefix map

gulfem created this revision. Herald added a project: All. gulfem requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch uses llvm::MapVector instead of std::map to ensure that iteration order is the insertion order for file prefix rema

[PATCH] D148757: [clang] Follow user-provided order for prefix map

gulfem added a comment. In D148757#4282002 , @MaskRay wrote: > Can you add example options in the description and say how this patch is > going to change it? > > I think `std::map` is intended so that for `-f*-prefix-map` values, if both > `a/b=...` and

[PATCH] D148757: [clang] Follow user-provided order for prefix map

gulfem updated this revision to Diff 515560. gulfem added a comment. Modified the test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 Files: clang/include/clang/Basic/CodeGenOptions.h clang/lib/

[PATCH] D148757: [clang] Follow user-provided order for prefix map

gulfem added a comment. I compared Clang and GCC behavior. **1) Following user-specified prefix mapping order** GCC uses a linked list to store the prefix mappings, so it applies the prefix mappings based on the user-provided order. https://github.com/gcc-mirror/gcc/blob/master/gcc/file-prefix-ma

[PATCH] D148757: [clang] Follow user-provided order for prefix map

gulfem updated this revision to Diff 516616. gulfem added a comment. Remove the changes from debuginfo, which is handled in D14897 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.or

[PATCH] D148757: [clang] Follow user-provided order for prefix map

gulfem added a comment. In D148757#4289152 , @MaskRay wrote: > In D148757#4289076 , @MaskRay wrote: > >> I'm creating a patch to change DebugPrefixMap to respect the command line >> option order... > > Created D1

[PATCH] D148757: [clang] Apply last -fcoverage-prefix-map option

gulfem updated this revision to Diff 516853. gulfem added a comment. Updated the text. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 Files: clang/include/clang/Basic/CodeGenOptions.h clang/lib/CodeG

[PATCH] D148757: [clang] Apply -fcoverage-prefix-map reverse order

gulfem updated this revision to Diff 516989. gulfem added a comment. Addressed feedback and rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 Files: clang/include/clang/Basic/CodeGenOptions.h c

[PATCH] D148757: [clang] Apply -fcoverage-prefix-map reverse order

gulfem updated this revision to Diff 516993. gulfem added a comment. Added a comment about reverse traversal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 Files: clang/include/clang/Basic/CodeGenOpti

[PATCH] D148757: [clang] Apply -fcoverage-prefix-map reverse order

gulfem marked 2 inline comments as done. gulfem added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1638 : CGM(CGM), SourceInfo(SourceInfo) { - CoveragePrefixMap = CGM.getCodeGenOpts().CoveragePrefixMap; + for (const auto &[From, To] : CGM.getCodeGe

  1   2   >