[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

rprichard added a comment. In D159292#4646096 , @smeenai wrote: > Ping. I asked a couple of other people at Google to take a look at the patches. Someone should get to it next week I think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

rprichard added a comment. This change looks like an improvement to me. The NDK currently puts the libc++ headers into the sysroot, but putting it in the usual toolchain include location seems better. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3108 +SmallString<128>

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

rprichard added inline comments. Comment at: compiler-rt/lib/builtins/cpu_model.c:1382 +return; +#if defined(__ANDROID__) + // ifunc resolvers don't have hwcaps in arguments on Android API lower enh wrote: > srhines wrote: > > MaskRay wrote: > > > enh

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

rprichard added a comment. > further confirmation if android_get_device_api_level should work from > ifunc_resolver IIRC an ifunc resolver in Bionic can't generally call any functions in libc, including `android_get_device_api_level` or `__system_property_get`, because the ifunc resolver will

[PATCH] D139503: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h

This revision was automatically updated to reflect the committed changes. Closed by commit rG106a99276f9e: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h (authored by rprichard). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139503/new/

[PATCH] D139503: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h

rprichard added a comment. As an aside, it seems interesting that libcxxabi/test/forced_unwind[12].pass.cpp don't compile for ARM EHABI when used with Clang's unwind.h. AFAICT it's been that way for a long time? $ /x/clang14/bin/clang -target arm-linux-gnueabi

[PATCH] D139503: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h

rprichard created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. rprichard requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use the same approach as used in libunwind/include/unwind_arm_ehabi.h (D89570

[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

rprichard accepted this revision. rprichard added a comment. This revision is now accepted and ready to land. It looks OK to me, but I wonder if you need someone else to accept it also. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135142/new/

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG02a25279aedc: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin (authored by rprichard). Repository: rG LLVM Github Monorepo

[PATCH] D127465: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG408a2638fda6: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386 (authored by rprichard). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

rprichard added subscribers: t.p.northover, EricWF, rsmith. rprichard edited reviewers, added: efriedma, craig.topper; removed: t.p.northover, EricWF, rsmith. rprichard added a comment. Herald added a subscriber: StephenFan. @efriedma @craig.topper Could you review this patch (or suggest someone

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

rprichard updated this revision to Diff 446302. rprichard added a comment. Stylistic change: keep the -ALIGN32 and -ALIGN64 suffixes in the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28213/new/ https://reviews.llvm.org/D28213 Files:

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard abandoned this revision. rprichard added a comment. Abandoning this patch because D127465 is a better fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127267/new/ https://reviews.llvm.org/D127267

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard added a comment. I uploaded D127465 , weakening the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127267/new/ https://reviews.llvm.org/D127267 ___

[PATCH] D127465: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386

rprichard added reviewers: tra, yaxunl. rprichard added a subscriber: mgorny. rprichard added a comment. This change is needed to keep the clang/test/Preprocessor/cuda-types.cu test passing after D28213 changes the value of `__CLANG_ATOMIC_LLONG_LOCK_FREE` from

[PATCH] D127465: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386

rprichard created this revision. Herald added subscribers: mattd, yaxunl. Herald added a project: All. rprichard requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The default host CPU for an i386 triple is typically at least an i586, which

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard added a subscriber: jlebar. rprichard added a comment. Ok, I can upload a patch omitting `*_ATOMIC_LLONG_LOCK_FREE` from the macro testing. FWIW, did you see this comment in clang/lib/Basic/Targets/NVPTX.cpp?: // This is a bit of a lie, but it controls __GCC_ATOMIC_XXX_LOCK_FREE,

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard added a comment. In D127267#3570269 , @yaxunl wrote: > This patch is to fix an issue, right? At least we need a test to prevent that > issue from happening again. Yes, this patch is necessary to keep the clang/test/Preprocessor/cuda-types.cu

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard added a comment. In D127267#3568388 , @rprichard wrote: > Aside: There are only two calls to AllocateTarget: one in > TargetInfo::CreateTargetInfo and one in NVPTXTargetInfo::NVPTXTargetInfo. > This change removes NVPTXTargetInfo's call.

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard added a comment. I don't expect this change to affect the compiler behavior by itself -- is there a particular test that should be written? The code that I'm moving into NVPTXTargetInfo::setAuxTarget is already tested via clang/test/Preprocessor/cuda-types.cu. Aside: There are only

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard added a comment. This change is needed for D28213 , so that the `__GCC_ATOMIC_LLONG_LOCK_FREE` macro matches for `-target i386-unknown-linux-gnu` between `--cuda-host-only` and `--cuda-device-only`. This is tested in

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

rprichard updated this revision to Diff 435037. rprichard edited the summary of this revision. rprichard added a comment. Rebase this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28213/new/ https://reviews.llvm.org/D28213 Files:

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

rprichard created this revision. Herald added subscribers: jsji, kosarev, mattd, gchakrabarti, asavonic, kerbowa, pengfei, jvesely. Herald added a project: All. rprichard requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, jholewinski. Herald added a reviewer:

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

rprichard added a comment. @mgorny Do you mind if I commandeer this Phabricator revision (and also D29542 , which I would abandon in favor of D59566 which is already merged)? I need this CL to get libc++ tests passing on

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

rprichard added subscribers: craig.topper, rprichard. rprichard added a comment. Herald added a project: All. Maybe this change is obsolete now that D59566 is merged? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D29542/new/

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

rprichard added a comment. In D96403#2639210 , @srhines wrote: > It is actually using `clang-r416183` from toolchains.py > . > From there, you

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

rprichard added a comment. In D96403#2638883 , @smeenai wrote: > With NDK r22, I only see libunwind.a for armv7. Will it be provided for other > architectures in future NDK versions? NDK r23 should have a libunwind.a for all architectures. This change

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa478b0a199f4: [Android] Default to --rtlib=compiler-rt (authored by rprichard). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

rprichard updated this revision to Diff 329475. rprichard added a comment. Rebase this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96404/new/ https://reviews.llvm.org/D96404 Files: clang/lib/Driver/ToolChains/Linux.cpp

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG91f8aacc040f: [Android] Use -l:libunwind.a with --rtlib=compiler-rt (authored by rprichard). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG729899f7b6bf: [libunwind] unw_* alias fixes for ELF and Mach-O (authored by rprichard). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

rprichard added a comment. The coff-dwarf.test buildbot failures were fixed by https://github.com/llvm/llvm-project/commit/0fd7c31a098efdfaa5a57dbac6e9c0921b00a999 (on Feb 11). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93003/new/

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

rprichard added a comment. For reference, I know of four ways that LLVM's libunwind is currently linked into programs: - Fuchsia/Windows (i.e. Clang's default behavior): it's part of libc++, e.g.: - Fuchsia: -

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

rprichard added a comment. Maybe this is blocked on someone from Apple reviewing the Mach-O parts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93003/new/ https://reviews.llvm.org/D93003 ___

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

rprichard updated this revision to Diff 323222. rprichard added a comment. Rebase and fix merge conflict in gn file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93003/new/ https://reviews.llvm.org/D93003 Files:

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

rprichard updated this revision to Diff 323221. rprichard edited the summary of this revision. rprichard added a comment. Update libunwind/src/BUILD.gn for macro name change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93003/new/

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

rprichard added reviewers: danalbert, srhines, pcc. rprichard added subscribers: thakis, glandium. rprichard added a comment. Adding glandium and thakis for Firefox and Chrome. e.g. I suspect this change would cause the same sort of breakage seen in D95166 .

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

rprichard created this revision. Herald added a subscriber: dberris. rprichard requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang. On Android, the unwinder isn't part of the C++ STL and isn't (in older versions) exported from libc.so.

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

rprichard created this revision. Herald added a subscriber: dberris. rprichard requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. By default, the driver uses the compiler-rt builtins and links with -l:libunwind.a. Restore the previous

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

rprichard added a subscriber: emaste. rprichard added a comment. emaste: Just a heads up in case FreeBSD is affected. FWIW, I noticed that libgcc_eh.a on FreeBSD 12.1 doesn't hide the `_Unwind_*` or `unw_*` symbols. The build system

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

rprichard added inline comments. Comment at: libunwind/src/assembly.h:82 .globl SYMBOL_NAME(aliasname) SEPARATOR \ - WEAK_SYMBOL(aliasname) SEPARATOR \ + EXPORT_SYMBOL(SYMBOL_NAME(aliasname))

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

rprichard updated this revision to Diff 322574. rprichard edited the summary of this revision. rprichard added a comment. Restore Mach-O alias fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93003/new/ https://reviews.llvm.org/D93003 Files:

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

rprichard added inline comments. Comment at: libunwind/src/assembly.h:82 .globl SYMBOL_NAME(aliasname) SEPARATOR \ - WEAK_SYMBOL(aliasname) SEPARATOR \ + EXPORT_SYMBOL(SYMBOL_NAME(aliasname))

[PATCH] D93003: [libunwind] LIBUNWIND_HERMETIC_STATIC_LIBRARY fixes

rprichard added a comment. After renaming the CMake option, CMake prints a warning if I set the old option: CMake Warning: Manually-specified variables were not used by the project: LIBUNWIND_HERMETIC_STATIC_LIBRARY Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D93003: [libunwind] LIBUNWIND_HERMETIC_STATIC_LIBRARY fixes

rprichard updated this revision to Diff 321701. rprichard added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Just fix ELF and leave Mach-O as-is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93003/new/

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

rprichard added a comment. Ok, I'll run that command in the future, and I'll watch D80978 for updates. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81622/new/ https://reviews.llvm.org/D81622

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c4ce202267e: [Driver] Search computed sysroot for libc++ header paths (authored by rprichard). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81622/new/

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

rprichard updated this revision to Diff 270030. rprichard added a comment. Remove unnecessary `virtual` keyword. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81622/new/ https://reviews.llvm.org/D81622 Files:

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

rprichard added reviewers: srhines, danalbert, libc++, kristina. rprichard added a comment. FWIW, `computeSysRoot` is overridden by these toolchains: - Linux - MSP430ToolChain - MipsLLVMToolChain - RISCVToolChain I think this change restores the pre D69758

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

rprichard updated this revision to Diff 270009. rprichard added a comment. Reverse clang-format's change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81622/new/ https://reviews.llvm.org/D81622 Files: clang/include/clang/Driver/ToolChain.h

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

rprichard updated this revision to Diff 270005. rprichard added a comment. Rerun after installing clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81622/new/ https://reviews.llvm.org/D81622 Files:

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

rprichard created this revision. Herald added subscribers: cfe-commits, luismarques, apazos, sameer.abuasal, pzheng, s.egerton, lenary, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, MaskRay, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb.

[PATCH] D61931: [Driver] Use --android-tls for Android ARM/AArch64 when lld is used

rprichard added inline comments. Comment at: lib/Driver/ToolChains/Gnu.cpp:404 + const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ); + if (A && StringRef(A->getValue()).contains("lld")) +CmdArgs.push_back("--android-tls"); The logic used

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

rprichard added a comment. In https://reviews.llvm.org/D38599#899198, @danalbert wrote: > In https://reviews.llvm.org/D38599#899196, @howard.hinnant wrote: > > > Fwiw, I wrote this code. All of that "fallback" stuff was written to make > > customer code that was incorrect, but working on OS X

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

rprichard added a comment. Some relevant code links: - https://github.com/llvm-mirror/clang/blob/8c9bf999aa40ab6077b958b5edcf587b9d76ce24/lib/CodeGen/ItaniumCXXABI.cpp#L359 ==> iOS64CXXABI overrides shouldRTTIBeUnique to return false. -

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

rprichard added a comment. Here's the Clang bug I filed: https://bugs.llvm.org/show_bug.cgi?id=34907 Repository: rL LLVM https://reviews.llvm.org/D38599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

rprichard added a comment. I assume std::type_info::operator== also needs to be adjusted to compare string names? It looks like libc++'s version of the function does string comparisons for ARM64 iOS, but only on some classes (e.g. public(?) ones). Look for the _LIBCPP_HAS_NONUNIQUE_TYPEINFO