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

srhines 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 MaskRay wrote: > enh wrote: > > ilinpv wrote: > > > MaskRay

[PATCH] D158476: [driver] Search for compatible Android runtime directories

srhines added a comment. This approach LGTM, assuming that https://reviews.llvm.org/D140925 lands as well to ensure that the triple does use `androideabi`, since it would prevent custom build setups from needing additional help. > (Aside: why do we need to produce runtime libraries for

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

srhines added a comment. > They'd already have had a chance to deal with their code when this was a > warning-default-error without "ShowInSystemHeaders"? (or, if the yhaven't > picked up a new compiler often enough, and they go from "a warning we didn't > care about" to

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

srhines accepted this revision. srhines added a comment. Thank you, Kees and reviewers, for helping to make progress here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142

[PATCH] D127528: [Clang] Let the linker choose shared or static libunwind unless specified

srhines added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1477 if (Args.hasArg(options::OPT_shared_libgcc)) return LibGccType::SharedLibGcc; - // The Android NDK only provides libunwind.a, not libunwind.so. phosek wrote: >

[PATCH] D116753: Default to -fno-math-errno for musl too

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Verified with the docs from musl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116753/new/ https://reviews.llvm.org/D116753

[PATCH] D114036: Add Android test case for -Wpartial-availability. Also update Android availability tests to match on the whole string, so we can distinguish between "Android 16" and "Android 16.0.0"

srhines accepted this revision. srhines added a comment. Thanks for the expanded (and more specific) tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114036/new/ https://reviews.llvm.org/D114036 ___

[PATCH] D99996: [Driver] Drop $DEFAULT_TRIPLE-$name as a fallback program name

srhines added a comment. Adding Dan as an FYI. While this doesn't impact Android platform or regular NDK users, I suppose that someone could have some esoteric build rules that are relying on this, but they should probably be more explicit about what they're running in those cases anyways.

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

srhines added a comment. In D96403#2639180 , @srhines wrote: > In D96403#2639137 , @smeenai wrote: > >> In D96403#2638932 , @rprichard >> wrote: >> >>> In D96403#2638883

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

srhines added a comment. In D96403#2639137 , @smeenai wrote: > In D96403#2638932 , @rprichard wrote: > >> In D96403#2638883 , @smeenai wrote: >> >>> With NDK r22, I only see

[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

srhines added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1911 + SmallVector Prefixes; + if (TargetTriple.isAndroid()) +Prefixes.assign(D.PrefixDirs.begin(), D.PrefixDirs.end()); danalbert wrote: > I'm not entirely sure what

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

srhines added a comment. In D83144#2268760 , @fmayer wrote: > Thanks for the review! Should I wait for Alex to take a look, or is it fine > like this? George added @alexfh a while ago, but if it's ok with you, maybe give him until Tuesday or Wednesday

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for creating the new test, and for making this more flexible. Everything else looks good here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83144/new/

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

srhines added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/android-comparison-in-temp-failure-retry.c:1 -// RUN: %check_clang_tidy %s android-comparison-in-temp-failure-retry %t +// RUN: %check_clang_tidy %s android-comparison-in-temp-failure-retry %t

[PATCH] D83726: Fix a missing update that C compiles default to gnu17.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9d5a8b7edb28: Fix a missing update that C compiles default to gnu17. (authored by srhines). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83726/new/

[PATCH] D83726: Fix a missing update that C compiles default to gnu17.

srhines created this revision. srhines added a reviewer: nickdesaulniers. Herald added a project: clang. Herald added a subscriber: cfe-commits. https://reviews.llvm.org/D75383 switched the C default to gnu17, but missed this instance. Repository: rG LLVM Github Monorepo

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

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks, Ryan! 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

srhines added a comment. Thanks, Ryan, for diagnosing and addressing this bug. Comment at: clang/lib/Driver/ToolChains/Linux.h:45 llvm::opt::ArgStringList ) const override; - virtual std::string computeSysRoot() const; + virtual std::string

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks, Nick, for cleaning this up and always striving to make things more compatible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

srhines added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:565 +case llvm::Triple::thumbeb: + if (Triple.getEnvironment() == llvm::Triple::Android) +return true; Triple.isAndroid() is more clear. Comment

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

srhines added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:560 // Don't use a frame pointer on linux if optimizing for certain targets. +case llvm::Triple::arm: +case llvm::Triple::armeb: For the new targets, we should only be

[PATCH] D77168: Add a flag to debug automatic variable initialization

srhines added a comment. In D77168#1988122 , @jfb wrote: > In D77168#1988049 , @srhines wrote: > > > `pragma clang attribute` is interesting, but how do you apply that in a > > selective fashion to local variables

[PATCH] D77168: Add a flag to debug automatic variable initialization

srhines added a comment. `pragma clang attribute` is interesting, but how do you apply that in a selective fashion to local variables (especially in a way that can be automated)? At first, I didn't think the goal for this should be to create a frequently used option for **most** end users, but

[PATCH] D77746: [Driver] Default arm-linux-androideabi to -z max-page-size=4096

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thank you for making this clear. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77746/new/ https://reviews.llvm.org/D77746

[PATCH] D77168: Add a flag to debug automatic variable initialization

srhines added a comment. In D77168#1955312 , @jfb wrote: > Do you not think `pragma` is a more general approach? That's what's used in a > bunch of other cases, and I'd like to see it attempted here. > If not, I agree with John that just counting up

[PATCH] D77168: Add a flag to debug automatic variable initialization

srhines added a comment. In D77168#1953635 , @jfb wrote: > I'm not sure this is a good idea at all. We want to keep the codepath as > simple as possible to avoid introducing bugs. If a codebase sees a crash then > it's easier to bisect one function at a

[PATCH] D76452: Use LLD by default for Android.

srhines added a comment. In D76452#1945029 , @MaskRay wrote: > In D76452#1944786 , @srhines wrote: > > > In D76452#1944733 , @MaskRay wrote: > > > > > Another approach will

[PATCH] D76452: Use LLD by default for Android.

srhines added a comment. In D76452#1944733 , @MaskRay wrote: > Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a default > value for `-fuse-ld=`. How does that work for Darwin builds? Is lld fully supported for MachO at this point,

[PATCH] D76452: Use LLD by default for Android.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks, Dan, for setting this up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76452/new/ https://reviews.llvm.org/D76452

[PATCH] D69925: remove redundant LLVM version from version string when setting CLANG_VENDOR

srhines added a comment. In D69925#1736434 , @efriedma wrote: > I think the reason it's written this way is that CLANG_VERSION_STRING might > not correspond to an LLVM version? For example, Apple has its own versioning > scheme. Line 128 is already

[PATCH] D67200: Add -static-openmp driver option

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Looks really nice. I am sure the NDK developers will be happy to see support for static OpenMP. Do you want to add the public NDK github issue link in the commit message? Repository: rG

[PATCH] D64666: [Sema] Enable -Wimplicit-int-float-conversion for integral to floating point precision loss

srhines added a comment. In D64666#1650325 , @sylvestre.ledru wrote: > @ziangwan maybe you should add this improvement to the release notes, wdyt? @sylvestre.ledru It's really great to hear that this new diagnostic is helpful. Thanks for the great

[PATCH] D64666: Allow Clang -Wconversion, -Wimplicit-float-conversion warns about integer type -> floating point type implicit conversion precision loss.

srhines added a comment. You have patch.patch in the diff above. You should drop that. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64666/new/ https://reviews.llvm.org/D64666 ___ cfe-commits mailing list

[PATCH] D64655: [Clang][Driver] don't error for unsupported as options for -no-integrates-as

srhines added a comment. s/integrates/integrated in the title, but otherwise this looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64655/new/ https://reviews.llvm.org/D64655 ___ cfe-commits

[PATCH] D63774: android: enable double-word CAS on x86_64

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Craig, can you confirm that this is acceptable? I don't think there are any chips with SSE4.2 but without cx16, so this just seemed like an oversight. It might be a good idea to really

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

srhines added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54 + +void e() { + int pipefd[2]; gribozavr wrote: > How is `e` different from `a`? `e` uses O_CLOEXEC properly with `pipe2()` and makes sure that we don't

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

srhines added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:17 + pipe(pipefd); + // CHECK-MESSAGES-NOT: warning: +} hokein wrote: > nit: no need to do it explicitly, if a warning is shown unexpectedly, the > test will

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Everything looks great. Thanks for adding these improvements. While it's probably safe to commit this, perhaps you should give it 24 hours in case some of the other clang-tidy folks have

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

srhines added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:48 + pipe2(pipefd, O_NONBLOCK); + // CHECK-MESSAGES-NOT: warning: + TEMP_FAILURE_RETRY(pipe2(pipefd, O_NONBLOCK)); Much like line 39 (which covers both

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

srhines added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:52 + +void e() { + int pipefd[2]; jcai19 wrote: > srhines wrote: > > I'm not all that familiar with writing clang-tidy-specific tests, but > > should these

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for picking this up and finishing it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57930/new/ https://reviews.llvm.org/D57930

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

srhines added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:52 + +void e() { + int pipefd[2]; I'm not all that familiar with writing clang-tidy-specific tests, but should these tests here denote that a diagnostic

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

srhines added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:19 + + pipe2(pipefd, O_CLOEXEC); Shouldn't this be "O_NONBLOCK | O_CLOEXEC" instead? Why drop the O_NONBLOCK? Repository: rG LLVM Github Monorepo

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

srhines added a comment. This LGTM if Ryan is happy with it. Thanks for taking care of getting a workaround implemented until this can be fixed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61931/new/ https://reviews.llvm.org/D61931

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357713: Verify that Android targets generate DWARF 4 by default. (authored by srhines, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

srhines marked an inline comment as done. srhines added inline comments. Comment at: clang/test/Driver/debug-options.c:280 // G_STANDALONE: "-debug-info-kind=standalone" +// G_DWARF2: "-dwarf-version=2" // G_DWARF4: "-dwarf-version=4" aprantl wrote: > What's

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

srhines created this revision. srhines added a reviewer: aprantl. Herald added a project: clang. Herald added a subscriber: cfe-commits. srhines added subscribers: pirama, chh. In the future, Android releases will support DWARF 5, but we need to ensure that older targets only have DWARF 4

[PATCH] D60059: [Driver] implement -feverything

srhines added a comment. In addition to -fno-everything, don't forget about -fnothing and -fno-nothing, which seem like they would also be nice to have. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60059/new/ https://reviews.llvm.org/D60059

[PATCH] D58531: [clang] Specify type of pthread_create builtin

srhines added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9574 - assert(!RequiresICE && "Result of intrinsic cannot be required to be an ICE"); + assert(!RequiresICE && "Result of function cannot be required to be an ICE"); I would just

[PATCH] D58477: [Driver] Fix float ABI default for Android ARMv8.

srhines accepted this revision. srhines added a comment. Dan, this seems pretty important for the NDK. If you submit this, would you want it cherry-picked ASAP? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58477/new/

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

srhines added a comment. Would it be reasonable to have a test for this with perhaps an invalid GCC installation? There is some mock GCC/sysroot testing in https://github.com/llvm/llvm-project/blob/master/clang/test/Driver/android-gcc-toolchain.c and

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

This revision was automatically updated to reflect the committed changes. Closed by commit rL353318: Switch to cantFail(), since it does the same assertion. (authored by srhines, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. In D36806#1385936 , @lhames wrote: > Looks like this was LGTM'd but never applied. Stephen -- do you have commit > access? Yeah, I was waiting on someone with approval for this area of the code to comment and then lost track

[PATCH] D55953: Android is not GNU, so don't claim that it is.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Sorry about the delay. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55953/new/ https://reviews.llvm.org/D55953

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

srhines added inline comments. Comment at: test/Driver/function-sections.c:77 // RUN: | FileCheck --check-prefix=CHECK-NOUS %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\ There should ideally be a test for the default behavior

[PATCH] D53850: Declares __cpu_model as dso local

srhines added a comment. Craig, does this look ok now? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53850/new/ https://reviews.llvm.org/D53850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55029: set default max-page-size to 4KB in lld for Android Aarch64

srhines added a comment. In D55029#1312167 , @ruiu wrote: > I wouldn't rush to submit this now, given that this issue is not new at all. > Maybe we can just wait for Peter's response? Sure, that's fine too. I misunderstood the "tangent" part of your

[PATCH] D55029: set default max-page-size to 4KB in lld for Android Aarch64

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks Zhizhou! I'm curious about Peter's answer to the 64KiB default as well, but I think we should move forward with getting this patch submitted. Repository: rC Clang CHANGES SINCE

[PATCH] D53463: [Driver] allow Android triples to alias for non Android targets

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Please remove the reference to b/X in the commit message. LLVM doesn't allow internal bug numbers, and you described the issue well enough without it. Repository: rC Clang

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

srhines added a comment. This LGTM, but we should wait to hear from Kristof before submitting. Repository: rC Clang https://reviews.llvm.org/D53121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53109: [Driver] Default Android toolchains to libc++.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Really cool! Thanks for making everything easier to use out-of-the-box. Repository: rC Clang https://reviews.llvm.org/D53109 ___

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

srhines added inline comments. Comment at: lib/Sema/SemaType.cpp:1682 // or via one or more typedefs." -if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus -&& TypeQuals & Result.getCVRQualifiers()) { This is broken for C11 and C17 (even

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

srhines added a comment. Thanks @dblaikie for the quick fixup. I must have accidentally dropped the '!', because I did run check-all to test the change. Repository: rL LLVM https://reviews.llvm.org/D52191 ___ cfe-commits mailing list

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

This revision was automatically updated to reflect the committed changes. Closed by commit rC342501: Fix logic around determining use of frame pointer with -pg. (authored by srhines, committed by ). Changed prior to commit: https://reviews.llvm.org/D52191?vs=165826=166007#toc Repository: rL

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

This revision was automatically updated to reflect the committed changes. Closed by commit rL342501: Fix logic around determining use of frame pointer with -pg. (authored by srhines, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52191

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

srhines added a comment. In https://reviews.llvm.org/D52191#1238628, @dblaikie wrote: > Sure, looks good. Though my other/vague concern is why does this case error > about fomit-frame-pointer having no effect, but other things (like using > -fomit-frame-pointer on a target that requires frame

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

srhines created this revision. srhines added a reviewer: dblaikie. Herald added a subscriber: cfe-commits. As part of r342165, I rewrote the logic to check whether -fno-omit-frame-pointer was passed after a -fomit-frame-pointer argument. This CL switches that logic to use the consolidated

[PATCH] D51713: Support -fno-omit-frame-pointer with -pg.

This revision was automatically updated to reflect the committed changes. Closed by commit rL342165: Support -fno-omit-frame-pointer with -pg. (authored by srhines, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51713 Files:

[PATCH] D51713: Support -fno-omit-frame-pointer with -pg.

srhines added a comment. In https://reviews.llvm.org/D51713#1232414, @manojgupta wrote: > What is the call generated with -pg for AMR32, __gnu_mcount_nc or _mount? > __gnu_mcount_nc with "-pg" is known to be broken ( > https://bugs.llvm.org/show_bug.cgi?id=33845) I CCed myself on that issue

[PATCH] D51713: Support -fno-omit-frame-pointer with -pg.

srhines added a comment. http://b/32510864 was the internal bug request, so I am noting it here for future reference, but I think that the patch itself is pretty self-explanatory (rather than filing a distinct upstream bug about this issue). Repository: rC Clang

[PATCH] D51713: Support -fno-omit-frame-pointer with -pg.

srhines added a comment. This was discovered in the Android build system (which passes -fomit-frame-pointer by default for ARM configurations. This leads to the inability to specify -pg, since there is no way to override the mere presence of -fomit-frame-pointer. Repository: rC Clang

[PATCH] D51713: Support -fno-omit-frame-pointer with -pg.

srhines created this revision. Herald added a subscriber: cfe-commits. srhines added a subscriber: nickdesaulniers. Previously, any instance of -fomit-frame-pointer would make it such that -pg was an invalid flag combination. If -fno-omit-frame-pointer is passed later on the command line (such

[PATCH] D51521: Refactor Addlibgcc to make the when and what logic more straightfoward.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for cleaning this up and adding better checks for Android. :) Repository: rC Clang https://reviews.llvm.org/D51521 ___ cfe-commits

[PATCH] D51068: [Android] Default to -fno-math-errno

srhines accepted this revision. srhines added inline comments. This revision is now accepted and ready to land. Comment at: lib/Driver/ToolChains/Linux.cpp:913 +return false; + return Generic_ELF::IsMathErrnoDefault(); +} pirama wrote: > I tried to be

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

srhines abandoned this revision. srhines added a comment. Removed this in favor of the suggestions here. Setting the CMAKE_FIND_ROOT_PATH_MODE* variables does make this properly hermetic. Repository: rC Clang https://reviews.llvm.org/D48459 ___

[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library

srhines added a comment. LGTM. Thanks for making the checking more clear. This should help bridge the gap for now, so that we can resume getting per-library profile data in Android. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list

[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library

srhines added a comment. Thanks for picking this up again and updating the change to add llvm_gcov_flush(). Comment at: test/profile/Inputs/instrprof-dlopen-dlclose-main.c:20 + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) {

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

srhines added a comment. In https://reviews.llvm.org/D48459#1143012, @smeenai wrote: > Actually, I would imagine that if you're cross-compiling or using a custom > sysroot, you should probably also specify CMAKE_FIND_ROOT_PATH and set > CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to ONLY, to limit all

[PATCH] D45454: Make __gcov_flush visible outside a shared library

srhines added a comment. In https://reviews.llvm.org/D45454#1144099, @davidxl wrote: > GCC's behavior is not documented and it also has changed over the years. > > Unless there is a bug, changing LLVM's gcov_flush visibility can potentially > break clients that depend on this behavior. I

[PATCH] D48570: [Driver] Do not add -lpthread & -lrt with -static-libsan on Android

srhines accepted this revision. srhines added a comment. Agree. This is the first time anyone is linking against static sanitizers on Android, so this is just something that we missed updating in the past. Repository: rC Clang https://reviews.llvm.org/D48570

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

srhines added a comment. Rui, I added you to this review (and the other corresponding one), as you reviewed ecbeckmann's original work here. Can you take a quick look at these? Thanks. Repository: rC Clang https://reviews.llvm.org/D48459 ___

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

srhines created this revision. Herald added a subscriber: mgorny. If we ignore these variables, we end up always using the host information for libxml2, when it is not appropriate. This makes builds less hermetic than they should ideally be. Repository: rC Clang

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

srhines added a comment. In https://reviews.llvm.org/D47894#1125728, @jyknight wrote: > In https://reviews.llvm.org/D47894#1125653, @efriedma wrote: > > > The problem would come from propagating nonnull-ness from something which > > isn't inherently nonnull. For example, strlen has a nonnull

[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.

srhines added a comment. Looks fine. Please wait for a more experienced person to review this, however. Thanks for adding this feature. Repository: rC Clang https://reviews.llvm.org/D46300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45291: [Driver] Infer Android sysroot location.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for adding this simplified support. Sorry about the extreme delay in getting these reviewed. Repository: rC Clang https://reviews.llvm.org/D45291

[PATCH] D44995: [Driver] Include the Android multiarch includes.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for adding this support. Sorry about the delay in reviewing. Repository: rC Clang https://reviews.llvm.org/D44995 ___ cfe-commits

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

srhines added a comment. Peter also requested that a test be added to make sure that rY was not accepted by the Clang assembler as a true synonym for xY. Comment at: lib/Basic/Targets/AArch64.cpp:320 +{{"r24"}, "x24"}, {{"r25"}, "x25"}, {{"r26"}, "x26"}, {{"r27"}, "x27"},

[PATCH] D44229: Don't use -pie in relocatable link.

srhines accepted this revision. srhines added a comment. This revision is now accepted and ready to land. Thanks for fixing this quickly. :) https://reviews.llvm.org/D44229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42676: Add support for LLVM_REPOSITORY_STRING.

srhines abandoned this revision. srhines added a comment. I'm going to solve this a different way. Repository: rC Clang https://reviews.llvm.org/D42676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43203: [Driver] Generate .eh_frame_hdr for static executables too.

srhines added a comment. LGTM, but we should make sure that there are no objections, especially since there is no rationale for why this was present to begin with. Repository: rC Clang https://reviews.llvm.org/D43203 ___ cfe-commits mailing

[PATCH] D42676: Add support for LLVM_REPOSITORY_STRING.

srhines created this revision. Herald added subscribers: cfe-commits, hintonda, mgorny. srhines added a reviewer: beanz. Without this, vendor Clang builds can occasionally pick up an arbitrary mirror location for the repository (at least on Android builds). This allows vendor builds to be more

[PATCH] D40476: Switch kryo to use -mcpu=cortex-a57 when invoking the assembler

srhines added inline comments. Comment at: lib/Driver/ToolChains/Gnu.cpp:661 Arg *A; -if ((A = Args.getLastArg(options::OPT_mcpu_EQ)) && -StringRef(A->getValue()).equals_lower("krait")) - CmdArgs.push_back("-mcpu=cortex-a15"); -else -

[PATCH] D40476: Switch kryo to use -mcpu=cortex-a57 when invoking the assembler

srhines added inline comments. Comment at: lib/Driver/ToolChains/Gnu.cpp:660 +// of a cpu flag. +Arg *A = Args.getLastArg(options::OPT_mcpu_EQ); +if (A) { Is it better to sink A into the if condition again? Comment at:

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. Ping again. This is really trivial. https://reviews.llvm.org/D36806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. Ping. https://reviews.llvm.org/D36806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. Ping https://reviews.llvm.org/D36806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. Ping. https://reviews.llvm.org/D36806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. In https://reviews.llvm.org/D36806#856070, @lhames wrote: > I've added an optional ErrMsg argument to cantFail in r312066 - you can now > use this to preserve your explanatory text. Thank you for adding this! I updated the CL to make use of it.

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines updated this revision to Diff 113351. srhines added a comment. Switch to using ErrMsg in cantFail(). https://reviews.llvm.org/D36806 Files: lib/Tooling/Core/Replacement.cpp Index: lib/Tooling/Core/Replacement.cpp ===

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

srhines added a comment. Any other comments? https://reviews.llvm.org/D36806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

  1   2   >