[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] 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] 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] 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] 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 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] 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: [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] 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] 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 added a comment. > Would you be ok with reverting this change until I can sort that out, or can > we disable the pass for those targets until then? I will disable the pass for those targets for now. When the issue is resolved, I would like to enable it for those targets as well. ==

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

gulfem added a comment. > FYI, I pushed the fix for the aarch64-coff issue now (D99572 > , rGd5c5cf5ce8d921fc8c5e1b608c298a1ffa688d37 > ) and > pushed another commit to remove the code for disa

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

gulfem added a comment. This seems to break our `Fucshia` builds. If we cannot do a quick fix, we need to revert this patch. Please see the error message below: FAIL: Clang :: ClangScanDeps/vfsoverlay.cpp (1863 of 27822) TEST 'Clang :: ClangScanDeps/vfsoverlay.cpp' FAILED

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

gulfem added a comment. > @gulfem We're taking a look now. Could you please post a link to the full > build log, including the cmake flags used? This is the link to one of our failing builds: https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang-linux-arm64/b8849674814150864912/overview Here,

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

gulfem added a comment. > Thanks! We found the issue. I think I'll revert the patch, and will reland a > fixed version. Great, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92191/new/ https://reviews.llvm.org/D92191 __

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

gulfem added a comment. > This patch breaks a two stage build with LTO: Thanks for the report @nathanchance, and I'm looking at it now. Is this failure from one of the bots? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94355/new/ https://reviews.

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

gulfem added a comment. @nathanchance do you prefer me to revert the patch first as it might take me a while to investigate it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94355/new/ https://reviews.llvm.org/D94355 _

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

gulfem added a comment. > I would say if it is going to take longer than the end of the week to fix the > issue, it might be nice to have it reverted so that other people's builds > continue to work (I know a few people who build with full LTO in a two stage > configuration every week). Defini

[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] D94355: [Passes] Add relative lookup table converter pass

gulfem updated this revision to Diff 331139. gulfem added a comment. 1. Add no_relative_lookup_table.ll test case that checks the cases where relative lookup table should not be generated 2. Add comments about dso_local check and single use check Repository: rG LLVM Github Monorepo CHANGES S

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

gulfem marked 4 inline comments as done. gulfem added inline comments. Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:32 + // If lookup table has more than one user, + // do not generate a relative lookup table. + if (!GV.hasOneUse()) hans w

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

gulfem marked 2 inline comments as done. gulfem added inline comments. Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391 + +if (!TM.getTargetTriple().isArch64Bit()) + return false; + lebedev.ri wrote: > 1. But all tests are using `x86_64` trip

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

gulfem added a comment. @lebedev.ri do you have further comments? If not, I would like to submit this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94355/new/ https://reviews.llvm.org/D94355 ___ c

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

gulfem added inline comments. Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391 + +if (!TM.getTargetTriple().isArch64Bit()) + return false; + lebedev.ri wrote: > gulfem wrote: > > lebedev.ri wrote: > > > 1. But all tests are using `x86_64` tri

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG78a65cd945d0: [Passes] Add relative lookup table converter pass (authored by gulfem). Changed prior to commit: https://reviews.llvm.org/D94355?vs=331139&id=332445#toc Repository: rG LLVM Github Monor

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

gulfem added inline comments. Comment at: llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn:64 "PromoteMemoryToRegister.cpp", +"RelLookupTableConverter.cpp" "SSAUpdater.cpp", thakis wrote: > leonardchan wrote: > > Good that you added this,

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

  1   2   >