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

gulfem updated this revision to Diff 517012. gulfem marked an inline comment as done. gulfem added a comment. Added more info into HelpText and reversed prefix map in the first place to incorporate coworker feedback. GCC uses a linked list to store the prefix mappings in reverse order: https://gi

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

gulfem updated this revision to Diff 517375. gulfem added a comment. Add text to document that coverage prefix map should be traversed in reverse order. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 Fi

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

gulfem added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:211 + + /// Prefix replacement map for coverage. + llvm::SmallVector, 0> CoveragePrefixMap; MaskRay wrote: > MaskRay wrote: > > While adding a comment, clarify what coverage it

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

gulfem updated this revision to Diff 517400. gulfem added a comment. Extend the comment about the usage of CoveragePrefixMap. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 Files: clang/include/clang/B

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd7fa92126cc3: [clang] Apply -fcoverage-prefix-map reverse order (authored by gulfem). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

gulfem abandoned this revision. gulfem added a comment. Herald added a subscriber: wlei. Created a duplicate review by mistake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127040/new/ https://reviews.llvm.org/D127040

[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 added inline comments. Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:132 + // Place new instruction sequence after GEP. + Builder.SetInsertPoint(GEP); + Value *Index = GEP->getOperand(2); jrtc27 wrote: > This line causes the bug seen

[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] 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] D102531: PR45881: Properly use CXXThisOverride for templated lambda

gulfem added a comment. We recently enabled assertions in our Fuchsia builds, and our builds started to fail: FAILED: obj/src/lib/storage/vfs/cpp/libcpp.fuchsia_vfs.cc.o ../../../recipe_cleanup/clangLhcV7J/bin/clang++ -MD -MF obj/src/lib/storage/vfs/cpp/libcpp.fuchsia_vfs.cc.o.d -D_LIBCPP_

[PATCH] D102531: PR45881: Properly use CXXThisOverride for templated lambda

gulfem added a comment. @aaron.ballman would it be possible for you to review this patch, so we can fix our broken builds? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102531/new/ https://reviews.llvm.org/D102531

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

gulfem added a comment. We started seeing the following test failures in our Fuchsia builds after this patch: Failed Tests (10): libc++ :: libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp libc++ :: std/experimental/language.support/support.coroutines/c

[PATCH] D102531: PR45881: Properly use CXXThisOverride for templated lambda

gulfem added a comment. Any update on this review? Our builds are still broken. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102531/new/ https://reviews.llvm.org/D102531 ___ cfe-commits mailing list cfe

[PATCH] D102531: PR45881: Properly use CXXThisOverride for templated lambda

gulfem added a comment. > I'll commit this by the end of the day if @rsmith wasn't able to take a look. Sounds good, thank you very much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102531/new/ https://reviews.llvm.org/D102531 _

<    1   2