[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

nvm i found the file in one of the previous commits. 
https://github.com/llvm/llvm-project/pull/68735 for review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

@alexfanqi can you please upload the file (tsan_rtl_riscv64.S) with the patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/CMakeLists.txt:5
 append_list_if(COMPILER_RT_HAS_MSSE4_2_FLAG -msse4.2 TSAN_RTL_CFLAGS)
-append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=530
+append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=656
TSAN_RTL_CFLAGS)

jrtc27 wrote:
> hiraditya wrote:
> > dvyukov wrote:
> > > vitalybuka wrote:
> > > > Maybe this one is not needed after 
> > > > b31bd6d8046d01a66aa92993bacb56b115a67fc5
> > > Yes, is this needed? What function does have larger frame?
> > > If we increase the limit, other arches will slowly slinetly degrade too.
> > yeah probably not needed anymore. but if we need this we can just change 
> > this only for RISC-V?
> I would rather diagnose and fix whatever makes RISC-V code generation less 
> efficient
without this change we have one less failure. so i'll discard the changes to 
`-Wframe-larger-than=656` and commit the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/CMakeLists.txt:5
 append_list_if(COMPILER_RT_HAS_MSSE4_2_FLAG -msse4.2 TSAN_RTL_CFLAGS)
-append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=530
+append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=656
TSAN_RTL_CFLAGS)

dvyukov wrote:
> vitalybuka wrote:
> > Maybe this one is not needed after b31bd6d8046d01a66aa92993bacb56b115a67fc5
> Yes, is this needed? What function does have larger frame?
> If we increase the limit, other arches will slowly slinetly degrade too.
yeah probably not needed anymore. but if we need this we can just change this 
only for RISC-V?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Another test that failed:
compiler-rt/test/tsan/signal_thread.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

In D145214#4650928 , @alexfanqi wrote:

> In D145214#4650189 , @hiraditya 
> wrote:
>
>> I tested it on a licheepi board (riscv64 debian) and all the tests modified 
>> in this patch have passed. LGTM.
>
> Can you help me land this patch? I used to have commit access, but seems not 
> working atm.
>
>> There are other tsan failures but i think it is okay if they can be fixed in 
>> subsequent patches.
>
> Do you have a list of these failures? I will try to reproduce on my machine 
> and fix in later patch.

While testing compiler-rt/test/tsan/RISCV64Config only 
compiler-rt/test/tsan/mmap_lots.cpp failed. I couldn't run all the compiler-rt 
tests on Licheepi because it is very slow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.

I tested it on a licheepi board (riscv64 debian) and all the tests passed. LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/CMakeLists.txt:223
+elseif(arch MATCHES "riscv64")
+  add_asm_sources(TSAN_ASM_SOURCES
+tsan_rtl_riscv64.S

hiraditya wrote:
> I'm getting error with this line for some reason. Unknown cmake command 
> "add_asm_sources" see: D152102
replace `add_asm_sources` with `set` and it will work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: compiler-rt/lib/tsan/rtl/CMakeLists.txt:223
+elseif(arch MATCHES "riscv64")
+  add_asm_sources(TSAN_ASM_SOURCES
+tsan_rtl_riscv64.S

I'm getting error with this line for some reason. Unknown cmake command 
"add_asm_sources" see: D152102


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

ping: @jrtc27 @asb do you have any pending feedback for this patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145214: [TSAN] add support for riscv64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.
Herald added subscribers: wangpc, sunshaoce.

@alexfanqi are you still actively working on this? maybe we can move it to 
github then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145214/new/

https://reviews.llvm.org/D145214

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159338: [clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

not sure if this is the right way to fix these tests. The problem is if a 
device is constraiend, this will further slow down the device and create more 
backlogs. Can we allow a way to skip these tests based on a flag/environment 
variable?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159338/new/

https://reviews.llvm.org/D159338

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

In D158566#4633847 , @kadircet wrote:

> In D158566#4616417 , @ilya-biryukov 
> wrote:
>
>> Open question: I also feel like the best option here is to fix the tests, 
>> but I'm not sure how hard that would be. @sammccall any thoughts?
>> I suspect the particular tests are flaky is because they rely on timeouts, 
>> not sure it's easy to disentangle them. Therefore, some workaround seems 
>> reasonable
>
> FWIW, i've put some details in 
> https://github.com/llvm/llvm-project/issues/64964#issuecomment-1702249740 and 
> we had some previous discussions but unfortunately these tests have timeouts 
> as a "poor-mans-deadlock-detection". i don't think we can get rid of the 
> timeouts, without sacrificing that detection.
> I can't remember how misleading buildbot outputs were, when deadlocks 
> happened, before we introduced timeouts though. So one alternative is let the 
> buildbots hang instead.
>
> ---
>
> Regarding the approach in this patch, I don't feel strongly about it but I 
> don't think it's a good idea to let people build clangd, without testing it 
> on environments they care about. They might suppress legitimate issues. 
> (there's also some value though, e.g. maybe they already performed testing 
> before, and don't want to run tests again, but in such a scenario we've 
> non-check equivalents of targets to only run builds).

This option only allows more flexibility to the developers and by default it is 
on so anyone who doesn't care about switching off the test on constrained 
systems can continue to do so. Do you have specific concerns on why this will 
prevent developers from testing?

> Are you building clangd deliberately or is it just being pulled in via 
> check-clang-tools? If you don't want to ship clangd with your toolchain at 
> all, I think it's better to set `CLANG_ENABLE_CLANGD` to `OFF`.

We want to build clangd and it gets tested on a regular basis but the timeout 
happens on mac builds that are overloaded at times. I don't want to disable the 
build of clangd just because 1 test is failing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158566/new/

https://reviews.llvm.org/D158566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

> I'm marking as requiring changes mostly for the latter comment about 
> CLANG_INCLUDE_TESTS.

Added CLANG_INCLUDE_TESTS to the conditional. btw when CLANG_INCLUDE_TESTS is 
OFF, `ninja check-clang-tools` target doesn't get created so adding 
CLANG_INCLUDE_TESTS here didn't seem necessary, although tbh i'm not too 
familiar with the build system.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158566/new/

https://reviews.llvm.org/D158566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 553738.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158566/new/

https://reviews.llvm.org/D158566

Files:
  clang-tools-extra/clangd/CMakeLists.txt


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -208,13 +208,14 @@
   include(AddGRPC)
 endif()
 
-if(CLANG_INCLUDE_TESTS)
+if(CLANGD_INCLUDE_TESTS AND CLANG_INCLUDE_TESTS)
   add_subdirectory(test)
   add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is 
stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support 
for Clangd" OFF)
+option(CLANGD_INCLUDE_TESTS "Include Clangd tests" ON)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual 
installation.")
 
 add_subdirectory(index/remote)


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -208,13 +208,14 @@
   include(AddGRPC)
 endif()
 
-if(CLANG_INCLUDE_TESTS)
+if(CLANGD_INCLUDE_TESTS AND CLANG_INCLUDE_TESTS)
   add_subdirectory(test)
   add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
+option(CLANGD_INCLUDE_TESTS "Include Clangd tests" ON)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
 
 add_subdirectory(index/remote)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a subscriber: nridge.
hiraditya added a comment.

Added previously reported issues shared by @nridge


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158566/new/

https://reviews.llvm.org/D158566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 552543.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158566/new/

https://reviews.llvm.org/D158566

Files:
  clang-tools-extra/clangd/CMakeLists.txt


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -208,13 +208,14 @@
   include(AddGRPC)
 endif()
 
-if(CLANG_INCLUDE_TESTS)
+if(CLANGD_INCLUDE_TESTS)
   add_subdirectory(test)
   add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is 
stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support 
for Clangd" OFF)
+option(CLANGD_INCLUDE_TESTS "Include Clangd tests" ON)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual 
installation.")
 
 add_subdirectory(index/remote)


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -208,13 +208,14 @@
   include(AddGRPC)
 endif()
 
-if(CLANG_INCLUDE_TESTS)
+if(CLANGD_INCLUDE_TESTS)
   add_subdirectory(test)
   add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
+option(CLANGD_INCLUDE_TESTS "Include Clangd tests" ON)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
 
 add_subdirectory(index/remote)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158534: [clang-tidy] Disable trivially-destructible check for darwin

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya abandoned this revision.
hiraditya added a comment.

In D158534#4607719 , @PiotrZSL wrote:

> 626849c71e85d546a004cc91866beab610222194 
>  didn't 
> fix this issue already ?

ah you're right. that fixes it. i should have tested on lastest clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158534/new/

https://reviews.llvm.org/D158534

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158534: [clang-tidy] Disable trivially-destructible check for darwin

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: PiotrZSL, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
hiraditya requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

For fat binaries this test fails. 
https://github.com/llvm/llvm-project/issues/60304


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158534

Files:
  
clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp
@@ -3,6 +3,9 @@
 // RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -fix 
--
 // RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' 
-warnings-as-errors='-*,performance-trivially-destructible' --
 
+// For fat binaries this test fails: 
https://github.com/llvm/llvm-project/issues/60304
+// UNSUPPORTED: system-darwin
+
 struct TriviallyDestructible1 {
   int a;
 };


Index: clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/trivially-destructible.cpp
@@ -3,6 +3,9 @@
 // RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -fix --
 // RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -warnings-as-errors='-*,performance-trivially-destructible' --
 
+// For fat binaries this test fails: https://github.com/llvm/llvm-project/issues/60304
+// UNSUPPORTED: system-darwin
+
 struct TriviallyDestructible1 {
   int a;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157331: [clang] Implement C23

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/Headers/stdckdint.h:13
+
+#if defined(__GNUC__)
+#define ckd_add(R, A, B) __builtin_add_overflow((A), (B), (R))

aaron.ballman wrote:
> enh wrote:
> > ZijunZhao wrote:
> > > cor3ntin wrote:
> > > > aaron.ballman wrote:
> > > > > hiraditya wrote:
> > > > > > enh wrote:
> > > > > > > hiraditya wrote:
> > > > > > > > xbolva00 wrote:
> > > > > > > > > yabinc wrote:
> > > > > > > > > > enh wrote:
> > > > > > > > > > > enh wrote:
> > > > > > > > > > > > enh wrote:
> > > > > > > > > > > > > ZijunZhao wrote:
> > > > > > > > > > > > > > enh wrote:
> > > > > > > > > > > > > > > is this ever _not_ set for clang?
> > > > > > > > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdbool.h#L23
> > > > > > > > > > > > > > I think it is set?
> > > > > > > > > > > > > i get an error from
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > /tmp$ cat x.c
> > > > > > > > > > > > > #if defined(__GNUC__)
> > > > > > > > > > > > > #error foo
> > > > > > > > > > > > > #endif
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > regardless of whether i compile with -std=c11 or 
> > > > > > > > > > > > > -std=gnu11.
> > > > > > > > > > > > > neither -ansi nor -pedantic seem to stop it either.
> > > > > > > > > > > > it does look like it _should_ be possible to not have 
> > > > > > > > > > > > it set though? 
> > > > > > > > > > > > llvm/llvm-project/clang/lib/Frontend/InitPreprocessor.cpp
> > > > > > > > > > > >  has:
> > > > > > > > > > > > ```
> > > > > > > > > > > >   if (LangOpts.GNUCVersion != 0) {
> > > > > > > > > > > > // Major, minor, patch, are given two decimal 
> > > > > > > > > > > > places each, so 4.2.1 becomes
> > > > > > > > > > > > // 40201.
> > > > > > > > > > > > unsigned GNUCMajor = LangOpts.GNUCVersion / 100 / 
> > > > > > > > > > > > 100;
> > > > > > > > > > > > unsigned GNUCMinor = LangOpts.GNUCVersion / 100 % 
> > > > > > > > > > > > 100;
> > > > > > > > > > > > unsigned GNUCPatch = LangOpts.GNUCVersion % 100;
> > > > > > > > > > > > Builder.defineMacro("__GNUC__", Twine(GNUCMajor));
> > > > > > > > > > > > Builder.defineMacro("__GNUC_MINOR__", 
> > > > > > > > > > > > Twine(GNUCMinor));
> > > > > > > > > > > > Builder.defineMacro("__GNUC_PATCHLEVEL__", 
> > > > > > > > > > > > Twine(GNUCPatch));
> > > > > > > > > > > > Builder.defineMacro("__GXX_ABI_VERSION", "1002");
> > > > > > > > > > > > 
> > > > > > > > > > > > if (LangOpts.CPlusPlus) {
> > > > > > > > > > > >   Builder.defineMacro("__GNUG__", Twine(GNUCMajor));
> > > > > > > > > > > >   Builder.defineMacro("__GXX_WEAK__");
> > > > > > > > > > > > }
> > > > > > > > > > > >   }
> > > > > > > > > > > > ```
> > > > > > > > > > > /me wonders whether the right test here is actually `#if 
> > > > > > > > > > > __has_feature(__builtin_add_overflow)` (etc)...
> > > > > > > > > > > 
> > > > > > > > > > > but at this point, you definitely need an llvm person :-)
> > > > > > > > > > From 
> > > > > > > > > > https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins,
> > > > > > > > > >  we can check them with
> > > > > > > > > >  __has_builtin(__builtin_add_overflow) && 
> > > > > > > > > > __has_builtin(__builtin_sub_overflow) && 
> > > > > > > > > > __has_builtin(__builtin_mul_overflow).
> > > > > > > > > > I saw some code also checks if __GNUC__ >= 5:
> > > > > > > > > > 
> > > > > > > > > > // The __GNUC__ checks can not be removed until we depend 
> > > > > > > > > > on GCC >= 10.1
> > > > > > > > > > // which is the first version that returns true for 
> > > > > > > > > > __has_builtin(__builtin_add_overflow)
> > > > > > > > > > #if __GNUC__ >= 5 || __has_builtin(__builtin_add_overflow)
> > > > > > > > > > 
> > > > > > > > > > I guess we don't need to support real gcc using this header 
> > > > > > > > > > here. So maybe only checking __has_builtin is enough?
> > > > > > > > > > 
> > > > > > > > > > By the way, if __builtin_add_overflow may not appear on 
> > > > > > > > > > some targets, do we need to modify tests to specify triple 
> > > > > > > > > > like "-triple "x86_64-unknown-unknown"" in 
> > > > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/builtins-overflow.c#L5
> > > > > > > > > >  ?
> > > > > > > > > > 
> > > > > > > > > #ifndef __has_builtin // Optional of course.
> > > > > > > > >   #define __has_builtin(x) 0  // Compatibility with non-clang 
> > > > > > > > > compilers.
> > > > > > > > > #endif
> > > > > > > > > 
> > > > > > > > > ...
> > > > > > > > > #if __has_builtin(__builtin_trap)
> > > > > > > > >   __builtin_trap();
> > > > > > > > > #else
> > > > > > > > >   abort();
> > > > > > > > > #endif
> > > > > > > > > /me wonders whether the right test here is actually #if 
> > > > > > > > > __has_feature(__builtin_add_overflow) (etc)...
> > > > > > > > 
> > > > > > > > i think that should be added.
> > > > 

[PATCH] D157331: [clang] Implement C23

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/Headers/stdckdint.h:13
+
+#if defined(__GNUC__)
+#define ckd_add(R, A, B) __builtin_add_overflow((A), (B), (R))

enh wrote:
> hiraditya wrote:
> > xbolva00 wrote:
> > > yabinc wrote:
> > > > enh wrote:
> > > > > enh wrote:
> > > > > > enh wrote:
> > > > > > > ZijunZhao wrote:
> > > > > > > > enh wrote:
> > > > > > > > > is this ever _not_ set for clang?
> > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdbool.h#L23
> > > > > > > > I think it is set?
> > > > > > > i get an error from
> > > > > > > ```
> > > > > > > /tmp$ cat x.c
> > > > > > > #if defined(__GNUC__)
> > > > > > > #error foo
> > > > > > > #endif
> > > > > > > ```
> > > > > > > regardless of whether i compile with -std=c11 or -std=gnu11.
> > > > > > > neither -ansi nor -pedantic seem to stop it either.
> > > > > > it does look like it _should_ be possible to not have it set 
> > > > > > though? llvm/llvm-project/clang/lib/Frontend/InitPreprocessor.cpp 
> > > > > > has:
> > > > > > ```
> > > > > >   if (LangOpts.GNUCVersion != 0) {
> > > > > > // Major, minor, patch, are given two decimal places each, so 
> > > > > > 4.2.1 becomes
> > > > > > // 40201.
> > > > > > unsigned GNUCMajor = LangOpts.GNUCVersion / 100 / 100;
> > > > > > unsigned GNUCMinor = LangOpts.GNUCVersion / 100 % 100;
> > > > > > unsigned GNUCPatch = LangOpts.GNUCVersion % 100;
> > > > > > Builder.defineMacro("__GNUC__", Twine(GNUCMajor));
> > > > > > Builder.defineMacro("__GNUC_MINOR__", Twine(GNUCMinor));
> > > > > > Builder.defineMacro("__GNUC_PATCHLEVEL__", Twine(GNUCPatch));
> > > > > > Builder.defineMacro("__GXX_ABI_VERSION", "1002");
> > > > > > 
> > > > > > if (LangOpts.CPlusPlus) {
> > > > > >   Builder.defineMacro("__GNUG__", Twine(GNUCMajor));
> > > > > >   Builder.defineMacro("__GXX_WEAK__");
> > > > > > }
> > > > > >   }
> > > > > > ```
> > > > > /me wonders whether the right test here is actually `#if 
> > > > > __has_feature(__builtin_add_overflow)` (etc)...
> > > > > 
> > > > > but at this point, you definitely need an llvm person :-)
> > > > From 
> > > > https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins,
> > > >  we can check them with
> > > >  __has_builtin(__builtin_add_overflow) && 
> > > > __has_builtin(__builtin_sub_overflow) && 
> > > > __has_builtin(__builtin_mul_overflow).
> > > > I saw some code also checks if __GNUC__ >= 5:
> > > > 
> > > > // The __GNUC__ checks can not be removed until we depend on GCC >= 10.1
> > > > // which is the first version that returns true for 
> > > > __has_builtin(__builtin_add_overflow)
> > > > #if __GNUC__ >= 5 || __has_builtin(__builtin_add_overflow)
> > > > 
> > > > I guess we don't need to support real gcc using this header here. So 
> > > > maybe only checking __has_builtin is enough?
> > > > 
> > > > By the way, if __builtin_add_overflow may not appear on some targets, 
> > > > do we need to modify tests to specify triple like "-triple 
> > > > "x86_64-unknown-unknown"" in 
> > > > https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/builtins-overflow.c#L5
> > > >  ?
> > > > 
> > > #ifndef __has_builtin // Optional of course.
> > >   #define __has_builtin(x) 0  // Compatibility with non-clang compilers.
> > > #endif
> > > 
> > > ...
> > > #if __has_builtin(__builtin_trap)
> > >   __builtin_trap();
> > > #else
> > >   abort();
> > > #endif
> > > /me wonders whether the right test here is actually #if 
> > > __has_feature(__builtin_add_overflow) (etc)...
> > 
> > i think that should be added.
> > 
> > I guess we also need a with `__STDC_VERSION__ > 202000L`? in princple we'd 
> > have a C23 number for it but i'm not sure if that has been added to clang 
> > yet.
> > i think that should be added.
> 
> i was advising the opposite --- now this is a standard C23 feature, any 
> architectures where __builtin_*_overflow doesn't work need to be found and 
> fixed. and we'll do that quicker if we unconditionally expose these and (more 
> importantly!) run the tests.
> 
> > I guess we also need a with __STDC_VERSION__ > 202000L?
> 
> _personally_ i think that's silly because you can't hide the header file, so 
> it doesn't make any sense to just have it empty if someone's actually 
> #included it. we don't do this anywhere in bionic for example, for this 
> reason. but obviously that's an llvm decision, and it does look like the 
> other similar headers _do_ have this check, so, yeah, probably.
> i was advising the opposite -- now this is a standard C23 feature, any 
> architectures where __builtin

you're right. it seems like `__builtin_add_overflow` is expected to be defined 
by default 
(https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/builtins-overflow.c#L4).

> and it does look like the other similar headers _do_ have this check, so, 
> yeah, probably.

yeah, Several headers have checks 

[PATCH] D157331: [clang] Implement C23

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/test/Headers/stdckdint.cpp:1
+// RUN: %clang_cc1 -emit-llvm -fgnuc-version=4.2.1 -std=gnu++11 %s -o - | 
FileCheck %s
+

seems like we don't have a -std=gnu23, or -std=c23 standard flag for this in 
clang yet.

https://godbolt.org/z/7dKnGEWWE

we probably need it before testing stdckdint i guess?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157331/new/

https://reviews.llvm.org/D157331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157331: [clang] Implement C23

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/Headers/stdckdint.h:13
+
+#if defined(__GNUC__)
+#define ckd_add(R, A, B) __builtin_add_overflow((A), (B), (R))

xbolva00 wrote:
> yabinc wrote:
> > enh wrote:
> > > enh wrote:
> > > > enh wrote:
> > > > > ZijunZhao wrote:
> > > > > > enh wrote:
> > > > > > > is this ever _not_ set for clang?
> > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdbool.h#L23
> > > > > > I think it is set?
> > > > > i get an error from
> > > > > ```
> > > > > /tmp$ cat x.c
> > > > > #if defined(__GNUC__)
> > > > > #error foo
> > > > > #endif
> > > > > ```
> > > > > regardless of whether i compile with -std=c11 or -std=gnu11.
> > > > > neither -ansi nor -pedantic seem to stop it either.
> > > > it does look like it _should_ be possible to not have it set though? 
> > > > llvm/llvm-project/clang/lib/Frontend/InitPreprocessor.cpp has:
> > > > ```
> > > >   if (LangOpts.GNUCVersion != 0) {
> > > > // Major, minor, patch, are given two decimal places each, so 4.2.1 
> > > > becomes
> > > > // 40201.
> > > > unsigned GNUCMajor = LangOpts.GNUCVersion / 100 / 100;
> > > > unsigned GNUCMinor = LangOpts.GNUCVersion / 100 % 100;
> > > > unsigned GNUCPatch = LangOpts.GNUCVersion % 100;
> > > > Builder.defineMacro("__GNUC__", Twine(GNUCMajor));
> > > > Builder.defineMacro("__GNUC_MINOR__", Twine(GNUCMinor));
> > > > Builder.defineMacro("__GNUC_PATCHLEVEL__", Twine(GNUCPatch));
> > > > Builder.defineMacro("__GXX_ABI_VERSION", "1002");
> > > > 
> > > > if (LangOpts.CPlusPlus) {
> > > >   Builder.defineMacro("__GNUG__", Twine(GNUCMajor));
> > > >   Builder.defineMacro("__GXX_WEAK__");
> > > > }
> > > >   }
> > > > ```
> > > /me wonders whether the right test here is actually `#if 
> > > __has_feature(__builtin_add_overflow)` (etc)...
> > > 
> > > but at this point, you definitely need an llvm person :-)
> > From 
> > https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins,
> >  we can check them with
> >  __has_builtin(__builtin_add_overflow) && 
> > __has_builtin(__builtin_sub_overflow) && 
> > __has_builtin(__builtin_mul_overflow).
> > I saw some code also checks if __GNUC__ >= 5:
> > 
> > // The __GNUC__ checks can not be removed until we depend on GCC >= 10.1
> > // which is the first version that returns true for 
> > __has_builtin(__builtin_add_overflow)
> > #if __GNUC__ >= 5 || __has_builtin(__builtin_add_overflow)
> > 
> > I guess we don't need to support real gcc using this header here. So maybe 
> > only checking __has_builtin is enough?
> > 
> > By the way, if __builtin_add_overflow may not appear on some targets, do we 
> > need to modify tests to specify triple like "-triple 
> > "x86_64-unknown-unknown"" in 
> > https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/builtins-overflow.c#L5
> >  ?
> > 
> #ifndef __has_builtin // Optional of course.
>   #define __has_builtin(x) 0  // Compatibility with non-clang compilers.
> #endif
> 
> ...
> #if __has_builtin(__builtin_trap)
>   __builtin_trap();
> #else
>   abort();
> #endif
> /me wonders whether the right test here is actually #if 
> __has_feature(__builtin_add_overflow) (etc)...

i think that should be added.

I guess we also need a with `__STDC_VERSION__ > 202000L`? in princple we'd have 
a C23 number for it but i'm not sure if that has been added to clang yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157331/new/

https://reviews.llvm.org/D157331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157331: [clang] Implement C23

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

For the record, the documentation for stdckdint.h is here: 
https://open-std.org/JTC1/SC22/WG14/www/docs/n3096.pdf#page=330


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157331/new/

https://reviews.llvm.org/D157331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157331: [clang] Implement C23

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/Headers/stdckdint.h:1
+/*=== stdckdint.h - Standard header for checking integer
+ *-===

nit: format.



Comment at: clang/test/Headers/stdckdint.cpp:1
+// RUN: %clang_cc1 -emit-llvm  -fgnuc-version=4.2.1 -std=gnu++11 %s -o - | 
FileCheck --check-prefix=CHECK-NEXT %s
+

1. nit: `-emit-llvm  -fgnuc-version=4.2.` has two spaces.
2. please chose a prefix other than `CHECK-NEXT`. as it has special meaning. 
https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-next-directive


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157331/new/

https://reviews.llvm.org/D157331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155339: Enable zba and zbs for RISCV64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/test/Driver/riscv-features.c:13
 
+// ANDROID: "-target-feature" "+zba"
 // ANDROID: "-target-feature" "+zbb"

MaskRay wrote:
> If the features are adjacent, test them on one line. `// ANDROID: 
> "-target-feature" "+zba" "-target-feature" "+zbb" ...`
that's a neat trick. it might work for zba, and zbb. But it could break for zbs 
if something else comes in between. that will make this test dependent on how 
flags are sorted internally. probably not worth micro-optimizing for this case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155339/new/

https://reviews.llvm.org/D155339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155339: Enable zba and zbs for riscv android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: enh, pirama, srhines.
Herald added subscribers: luke, VincentWu, danielkiss, vkmr, frasercrmck, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, 
arichardson.
Herald added a project: All.
hiraditya requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, eopXD, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155339

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -10,7 +10,9 @@
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck 
%s -check-prefix=RELAX
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mno-relax 2>&1 | 
FileCheck %s -check-prefix=NO-RELAX
 
+// ANDROID: "-target-feature" "+zba"
 // ANDROID: "-target-feature" "+zbb"
+// ANDROID: "-target-feature" "+zbs"
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -294,7 +294,7 @@
   return "rv32imafdc";
 else if (MABI.starts_with_insensitive("lp64")) {
   if (Triple.isAndroid())
-return "rv64imafdc_zbb";
+return "rv64imafdc_zba_zbb_zbs";
 
   return "rv64imafdc";
 }
@@ -314,7 +314,7 @@
 if (Triple.getOS() == llvm::Triple::UnknownOS)
   return "rv64imac";
 else if (Triple.isAndroid())
-  return "rv64imafdc_zbb";
+  return "rv64imafdc_zba_zbb_zbs";
 else
   return "rv64imafdc";
   }


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -10,7 +10,9 @@
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s -check-prefix=RELAX
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck %s -check-prefix=NO-RELAX
 
+// ANDROID: "-target-feature" "+zba"
 // ANDROID: "-target-feature" "+zbb"
+// ANDROID: "-target-feature" "+zbs"
 // RELAX: "-target-feature" "+relax"
 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -294,7 +294,7 @@
   return "rv32imafdc";
 else if (MABI.starts_with_insensitive("lp64")) {
   if (Triple.isAndroid())
-return "rv64imafdc_zbb";
+return "rv64imafdc_zba_zbb_zbs";
 
   return "rv64imafdc";
 }
@@ -314,7 +314,7 @@
 if (Triple.getOS() == llvm::Triple::UnknownOS)
   return "rv64imac";
 else if (Triple.isAndroid())
-  return "rv64imafdc_zbb";
+  return "rv64imafdc_zba_zbb_zbs";
 else
   return "rv64imafdc";
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

In D152279#4406896 , @MaskRay wrote:

> In D152279#4405901 , @asb wrote:
>
>> One of the key things we've been discussing on this at the LLVM call is that 
>> we probably want to keep the small data limit for embedded targets.
>
> It'd be useful to hear from some concrete embedded target users, whether they 
> customize `-msmall-data-limit=`, whether they are happy with 
> `-msmall-data-limit=8`, or whether they are just unaware of this threshold.
>
> If an embedded system typically customizes compiler driver options a lot, I 
> think they can consider adding `-msmall-data-limit=` (with a value suitable 
> for their use cases) in a configuration file 
> , not 
> letting their use cases dictate Linux systems.
>
> I putting up the patch partly came from my stance finding UX of this option 
> is really confusing and misleading. I wish that the embedded target users can 
> give me compelling arguments to keep `-msmall-data-limit=8` (and not another 
> value).

The default of 8 is probably to make it consistent with gcc. Here is text from 
man gcc 

  -msmall-data
  -mlarge-data
  When -mexplicit-relocs is in effect, static data is accessed
  via gp-relative relocations.  When -msmall-data is used,
  objects 8 bytes long or smaller are placed in a small data
  area (the ".sdata" and ".sbss" sections) and are accessed via
  16-bit relocations off of the $gp register.  This limits the
  size of the small data area to 64KB, but allows the variables
  to be directly accessed via a single instruction.
  
  The default is -mlarge-data.  With this option the data area
  is limited to just below 2GB.  Programs that require more
  than 2GB of data must use "malloc" or "mmap" to allocate the
  data in the heap instead of in the program's data segment.
  
  When generating code for shared libraries, -fpic implies
  -msmall-data and -fPIC implies -mlarge-data.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152279/new/

https://reviews.llvm.org/D152279

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

> Is there more context on why Android enables the frame pointer?

From what i gathered, this is more of an effort to have parity such that 
existing build flag overrides continue to be consistent.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 522252.
hiraditya added a comment.

Addressed comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,10 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 
FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | 
FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK0-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O1 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK1-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O2 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK2-32 %s
@@ -81,3 +85,5 @@
 // CHECK3-64-NOT: -mframe-pointer=all
 // CHECKs-64-NOT: -mframe-pointer=all
 // CHECK-MACHO-64: -mframe-pointer=all
+
+// CHECK-ANDROID-64: -mframe-pointer=non-leaf
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -106,6 +106,12 @@
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target ve-unknown-linux-gnu -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=aarch64-linux-android -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=aarch64-linux-android -S -O2 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=aarch64-linux-android -S -Os %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target powerpc64 -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
@@ -153,6 +159,9 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 
-S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
-
+// RUN: %clang -### --target=riscv64-linux-android -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### --target=riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -420,6 +420,20 @@
   if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
+  if (Triple.isAndroid()) {
+switch (Triple.getArch()) {
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::riscv64:
+  return true;
+default:
+  break;
+}
+  }
+
   switch (Triple.getArch()) {
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
@@ -459,9 +473,6 @@
 case llvm::Triple::armeb:
 case llvm::Triple::thumb:
 case llvm::Triple::thumbeb:
-  if (Triple.isAndroid())
-return true;
-  [[fallthrough]];
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
@@ -515,7 +526,8 @@
   bool OmitLeafFP =
   Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS() || Triple.isVE());
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && Triple.isRISCV64()));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
 if (OmitLeafFP)


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,10 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S 

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 521835.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 
FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | 
FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK0-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O1 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK1-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O2 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK2-32 %s
@@ -81,3 +87,5 @@
 // CHECK3-64-NOT: -mframe-pointer=all
 // CHECKs-64-NOT: -mframe-pointer=all
 // CHECK-MACHO-64: -mframe-pointer=all
+
+// CHECK-ANDROID-64: -mframe-pointer=non-leaf
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -106,6 +106,12 @@
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target ve-unknown-linux-gnu -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target aarch64-linux-android -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target aarch64-linux-android -S -O2 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target aarch64-linux-android -S -Os %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target powerpc64 -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
@@ -153,6 +159,9 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 
-S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
-
+// RUN: %clang -### -target riscv64-linux-android -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -420,6 +420,20 @@
   if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
+  if (Triple.isAndroid()) {
+switch (Triple.getArch()) {
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::riscv64:
+  return true;
+default:
+  break;
+}
+  }
+
   switch (Triple.getArch()) {
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
@@ -459,9 +473,6 @@
 case llvm::Triple::armeb:
 case llvm::Triple::thumb:
 case llvm::Triple::thumbeb:
-  if (Triple.isAndroid())
-return true;
-  [[fallthrough]];
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
@@ -515,7 +526,8 @@
   bool OmitLeafFP =
   Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS() || Triple.isVE());
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && (Triple.getArch() == 
llvm::Triple::riscv64)));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
 if (OmitLeafFP)


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 521834.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 
FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | 
FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK0-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O1 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK1-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O2 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK2-32 %s
@@ -81,3 +87,5 @@
 // CHECK3-64-NOT: -mframe-pointer=all
 // CHECKs-64-NOT: -mframe-pointer=all
 // CHECK-MACHO-64: -mframe-pointer=all
+
+// CHECK-ANDROID-64: -mframe-pointer=non-leaf
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -106,6 +106,8 @@
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target ve-unknown-linux-gnu -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target aarch64-linux-android -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target powerpc64 -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
@@ -153,6 +155,9 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 
-S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
-
+// RUN: %clang -### -target riscv64-linux-android -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -420,6 +420,20 @@
   if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
+  if (Triple.isAndroid()) {
+switch (Triple.getArch()) {
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::riscv64:
+  return true;
+default:
+  break;
+}
+  }
+
   switch (Triple.getArch()) {
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
@@ -459,9 +473,6 @@
 case llvm::Triple::armeb:
 case llvm::Triple::thumb:
 case llvm::Triple::thumbeb:
-  if (Triple.isAndroid())
-return true;
-  [[fallthrough]];
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
@@ -515,7 +526,8 @@
   bool OmitLeafFP =
   Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS() || Triple.isVE());
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && (Triple.getArch() == 
llvm::Triple::riscv64)));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
 if (OmitLeafFP)


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:424
+  if (Triple.isAndroid()) {
+// AArch64 has frame pointers enabled for non-leaf functions.
+switch (Triple.getArch()) {

hiraditya wrote:
> enh wrote:
> > (where? is it simpler to just add arm64 to the switch?)
> yeah we can add it for better readability but it would be redundant.
seems like we don't even have a test for aarch64.*android target. I'll add a 
testcase. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 521833.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 
FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | 
FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK0-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O1 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK1-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O2 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK2-32 %s
@@ -81,3 +87,5 @@
 // CHECK3-64-NOT: -mframe-pointer=all
 // CHECKs-64-NOT: -mframe-pointer=all
 // CHECK-MACHO-64: -mframe-pointer=all
+
+// CHECK-ANDROID-64: -mframe-pointer=non-leaf
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -153,6 +153,9 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 
-S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
-
+// RUN: %clang -### -target riscv64-linux-android -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -420,6 +420,20 @@
   if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
+  if (Triple.isAndroid()) {
+switch (Triple.getArch()) {
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::riscv64:
+  return true;
+default:
+  break;
+}
+  }
+
   switch (Triple.getArch()) {
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
@@ -459,9 +473,6 @@
 case llvm::Triple::armeb:
 case llvm::Triple::thumb:
 case llvm::Triple::thumbeb:
-  if (Triple.isAndroid())
-return true;
-  [[fallthrough]];
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
@@ -515,7 +526,8 @@
   bool OmitLeafFP =
   Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS() || Triple.isVE());
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && (Triple.getArch() == 
llvm::Triple::riscv64)));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
 if (OmitLeafFP)


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:424
+  if (Triple.isAndroid()) {
+// AArch64 has frame pointers enabled for non-leaf functions.
+switch (Triple.getArch()) {

enh wrote:
> (where? is it simpler to just add arm64 to the switch?)
yeah we can add it for better readability but it would be redundant.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:530
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && (Triple.getArch() == 
llvm::Triple::riscv64)));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||

enh wrote:
> how does this work for Android/arm64?
becaues of `Triple.isAArch64()`, AArch64 always has non-leaf frame pointers for 
all platforms.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 521826.
hiraditya added a comment.

Addressed comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150490/new/

https://reviews.llvm.org/D150490

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 
FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | 
FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK0-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O1 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK1-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O2 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK2-32 %s
@@ -81,3 +87,5 @@
 // CHECK3-64-NOT: -mframe-pointer=all
 // CHECKs-64-NOT: -mframe-pointer=all
 // CHECK-MACHO-64: -mframe-pointer=all
+
+// CHECK-ANDROID-64: -mframe-pointer=non-leaf
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -153,6 +153,9 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 
-S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
-
+// RUN: %clang -### -target riscv64-linux-android -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -420,6 +420,20 @@
   if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
+  if (Triple.isAndroid()) {
+// AArch64 has frame pointers enabled for non-leaf functions.
+switch (Triple.getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::riscv64:
+  return true;
+default:
+  break;
+}
+  }
+
   switch (Triple.getArch()) {
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
@@ -459,9 +473,6 @@
 case llvm::Triple::armeb:
 case llvm::Triple::thumb:
 case llvm::Triple::thumbeb:
-  if (Triple.isAndroid())
-return true;
-  [[fallthrough]];
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
@@ -515,7 +526,8 @@
   bool OmitLeafFP =
   Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS() || Triple.isVE());
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && (Triple.getArch() == 
llvm::Triple::riscv64)));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
 if (OmitLeafFP)


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck 

[PATCH] D150490: Enable frame pointer for all non-leaf functions on riscv64 Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: enh, danalbert, pirama, srhines.
Herald added subscribers: VincentWu, danielkiss, vkmr, sameer.abuasal, 
s.egerton, Jim, benna, psnobl, rogfer01, shiva0217, kito-cheng, simoncook, asb, 
arichardson.
Herald added a project: All.
hiraditya requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150490

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  clang/test/Driver/frame-pointer.c


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | 
FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | 
FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck 
-check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK0-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O1 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK1-32 %s
 // RUN: %clang --target=loongarch32 -### -S -O2 %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECK2-32 %s
@@ -81,3 +87,5 @@
 // CHECK3-64-NOT: -mframe-pointer=all
 // CHECKs-64-NOT: -mframe-pointer=all
 // CHECK-MACHO-64: -mframe-pointer=all
+
+// CHECK-ANDROID-64: -mframe-pointer=non-leaf
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -153,6 +153,9 @@
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
 // RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 
-S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
-
+// RUN: %clang -### -target riscv64-linux-android -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target riscv64-linux-android -mbig-endian -O1 -S %s 2>&1 
| \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -420,6 +420,9 @@
   if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
+  if (Triple.isAndroid() && Triple.getArch() == llvm::Triple::riscv64)
+return true;
+
   switch (Triple.getArch()) {
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
@@ -515,7 +518,8 @@
   bool OmitLeafFP =
   Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
options::OPT_mno_omit_leaf_frame_pointer,
-   Triple.isAArch64() || Triple.isPS() || Triple.isVE());
+   Triple.isAArch64() || Triple.isPS() || Triple.isVE() ||
+   (Triple.isAndroid() && (Triple.getArch() == 
llvm::Triple::riscv64)));
   if (NoOmitFP || mustUseNonLeafFramePointerForTarget(Triple) ||
   (!OmitFP && useFramePointerForTargetByDefault(Args, Triple))) {
 if (OmitLeafFP)


Index: clang/test/Driver/frame-pointer.c
===
--- clang/test/Driver/frame-pointer.c
+++ clang/test/Driver/frame-pointer.c
@@ -57,6 +57,12 @@
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK3-64 %s
 // RUN: %clang --target=riscv64-unknown-linux-gnu -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECKs-64 %s
 
+// RUN: %clang --target=riscv64-linux-android -### -S -O0 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O1 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O2 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -O3 %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+// RUN: %clang --target=riscv64-linux-android -### -S -Os %s 2>&1 | FileCheck -check-prefix=CHECK-ANDROID-64 %s
+
 // RUN: %clang --target=loongarch32 -### -S -O0 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECK0-32 %s
 

[PATCH] D149980: Remove unused basic_android_tree/mipsel-linux-android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: danalbert, enh, pirama, srhines.
Herald added subscribers: danielkiss, atanasyan, jrtc27, arichardson, sdardis.
Herald added a project: All.
hiraditya requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149980

Files:
  clang/test/Driver/Inputs/basic_android_tree/mipsel-linux-android/bin/.keep
  
clang/test/Driver/Inputs/basic_android_tree/mipsel-linux-android/include/c++/4.4.3/.keep
  clang/test/Driver/Inputs/basic_android_tree/mipsel-linux-android/lib/.keep




___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146565: [clang] Remove mips target triple for Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

llvm/test/CodeGen/Mips/ removed here: 
https://reviews.llvm.org/rG805f51f9fedf90d2aa0ad46c61cb4c9c0c5bcfe9


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146565/new/

https://reviews.llvm.org/D146565

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

https://lists.riscv.org/g/sig-toolchains/message/548 has a proposal where X27 
is one of the registers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146463/new/

https://reviews.llvm.org/D146463

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:62
-MF.getFunction(),
-"Shadow Call Stack cannot be combined with Save/Restore LibCalls."});
 return;

❤ 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146463/new/

https://reviews.llvm.org/D146463

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.

Thanks for putting the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146463/new/

https://reviews.llvm.org/D146463

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145999: [RISCV] Reserve X18 by default for Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 505378.
hiraditya added a comment.

Folded D145979  here


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145999/new/

https://reviews.llvm.org/D145999

Files:
  clang/test/Driver/sanitizer-ld.c
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll


Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -58,6 +58,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | 
FileCheck %s -check-prefix=X31
 
 ; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s 
-check-prefix=X18
+; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | 
FileCheck %s -check-prefix=X18
 
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -103,7 +103,7 @@
 
 bool isX18ReservedByDefault(const Triple ) {
   // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
-  return TT.isOSFuchsia();
+  return TT.isOSFuchsia() || TT.isAndroid();
 }
 
 } // namespace RISCV
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -743,6 +743,10 @@
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-RISCV64 %s
 // CHECK-SHADOWCALLSTACK-LINUX-RISCV64: '-fsanitize=shadow-call-stack' only 
allowed with '-ffixed-x18'
 
+// RUN: %clang -target riscv64-linux-android -fsanitize=shadow-call-stack %s 
-### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-SHADOWCALLSTACK-ANDROID-RISCV64
+// CHECK-SHADOWCALLSTACK-ANDROID-RISCV64-NOT: error:
+
 // RUN: %clang -fsanitize=shadow-call-stack -### %s 2>&1 \
 // RUN: --target=riscv64-unknown-fuchsia -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-FUCHSIA-RISCV64 %s


Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -58,6 +58,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
 ; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
 
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -103,7 +103,7 @@
 
 bool isX18ReservedByDefault(const Triple ) {
   // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
-  return TT.isOSFuchsia();
+  return TT.isOSFuchsia() || TT.isAndroid();
 }
 
 } // namespace RISCV
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -743,6 +743,10 @@
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-RISCV64 %s
 // CHECK-SHADOWCALLSTACK-LINUX-RISCV64: '-fsanitize=shadow-call-stack' only allowed with '-ffixed-x18'
 
+// RUN: %clang -target riscv64-linux-android -fsanitize=shadow-call-stack %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-SHADOWCALLSTACK-ANDROID-RISCV64
+// CHECK-SHADOWCALLSTACK-ANDROID-RISCV64-NOT: error:
+
 // RUN: %clang -fsanitize=shadow-call-stack -### %s 2>&1 \
 // RUN: --target=riscv64-unknown-fuchsia -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-FUCHSIA-RISCV64 %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145999: [RISCV] Reserve X18 by default for Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/test/Driver/riscv-fixed-x-register.c:343
+
+// Check that x18 is reserved on Android by default
+// RUN: %clang --target=riscv64-linux-android -### %s 2> %t

MaskRay wrote:
> MaskRay wrote:
> > asb wrote:
> > > samitolvanen wrote:
> > > > This seems redundant. Isn't the LLVM codegen test sufficient here?
> > > I think testing that the `+reserve-x18` target feature is added by the 
> > > frontend probably has some value.
> > > 
> > > I think it would better match the rest of this file to just do 
> > > `--check-prefix=CHECK-FIXED-X18` (I _think_ that works, but it is the end 
> > > of the day so I may be missing something obvious...).
> > > 
> > > Please do try to remember to generate patches with full context 
> > > (`-U99` or similar).
> > I prefer shared CHECK prefixes `--check-prefix=CHECK-FIXED-X18` as well. We 
> > do that for many other tests.
> `/// Check that x18 is reserved on Android by default.`
@samitolvanen correct, this test isn't possible because llvm implicitly 
reserves the register so clang -cc1 never gets the `"-target-feature" 
"+reserve-x18"` parameter.  I'll remove this test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145999/new/

https://reviews.llvm.org/D145999

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145999: [RISCV] Reserve X18 by default for Android

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: enh, asb.
Herald added subscribers: jobnoorman, luke, VincentWu, abrachet, danielkiss, 
vkmr, frasercrmck, evandro, phosek, luismarques, apazos, sameer.abuasal, 
s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar, arichardson.
Herald added a project: All.
hiraditya requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, eopXD, 
MaskRay.
Herald added projects: clang, LLVM.

Reserve X18 even when -fsanitize=shadow-call-stack is not enabled.

Based on: https://reviews.llvm.org/D143355


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145999

Files:
  clang/test/Driver/riscv-fixed-x-register.c
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll


Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -58,6 +58,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | 
FileCheck %s -check-prefix=X31
 
 ; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s 
-check-prefix=X18
+; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | 
FileCheck %s -check-prefix=X18
 
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -103,7 +103,7 @@
 
 bool isX18ReservedByDefault(const Triple ) {
   // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
-  return TT.isOSFuchsia();
+  return TT.isOSFuchsia() || TT.isAndroid();
 }
 
 } // namespace RISCV
Index: clang/test/Driver/riscv-fixed-x-register.c
===
--- clang/test/Driver/riscv-fixed-x-register.c
+++ clang/test/Driver/riscv-fixed-x-register.c
@@ -339,3 +339,8 @@
 // RUN: --check-prefix=CHECK-FIXED-X30 \
 // RUN: --check-prefix=CHECK-FIXED-X31 \
 // RUN: < %t %s
+
+// Check that x18 is reserved on Android by default
+// RUN: %clang --target=riscv64-linux-android -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ANDROID-FIXED-X18 < %t %s
+// CHECK-ANDROID-FIXED-X18: "-target-feature" "+reserve-x18"


Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -58,6 +58,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
 ; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
 
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -103,7 +103,7 @@
 
 bool isX18ReservedByDefault(const Triple ) {
   // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
-  return TT.isOSFuchsia();
+  return TT.isOSFuchsia() || TT.isAndroid();
 }
 
 } // namespace RISCV
Index: clang/test/Driver/riscv-fixed-x-register.c
===
--- clang/test/Driver/riscv-fixed-x-register.c
+++ clang/test/Driver/riscv-fixed-x-register.c
@@ -339,3 +339,8 @@
 // RUN: --check-prefix=CHECK-FIXED-X30 \
 // RUN: --check-prefix=CHECK-FIXED-X31 \
 // RUN: < %t %s
+
+// Check that x18 is reserved on Android by default
+// RUN: %clang --target=riscv64-linux-android -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ANDROID-FIXED-X18 < %t %s
+// CHECK-ANDROID-FIXED-X18: "-target-feature" "+reserve-x18"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143692: [clang][driver] Emit error when enabling emulated tls on unsupported architectures

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.

LGTM, thanks for putting this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143692/new/

https://reviews.llvm.org/D143692

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131230: [RISCV] Allow mismatched SmallDataLimit and use Min for conflicting values

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 495641.
hiraditya added a comment.

Updated the failing testcase.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131230/new/

https://reviews.llvm.org/D131230

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c


Index: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c
@@ -29,11 +29,11 @@
 //.
 // RV32: !0 = !{i32 1, !"wchar_size", i32 4}
 // RV32: !1 = !{i32 1, !"target-abi", !"ilp32d"}
-// RV32: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV32: !2 = !{i32 8, !"SmallDataLimit", i32 0}
 // RV32: !3 = !{!"vlenb"}
 //.
 // RV64: !0 = !{i32 1, !"wchar_size", i32 4}
 // RV64: !1 = !{i32 1, !"target-abi", !"lp64d"}
-// RV64: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV64: !2 = !{i32 8, !"SmallDataLimit", i32 0}
 // RV64: !3 = !{!"vlenb"}
 //.
Index: clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
===
--- clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
+++ clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
@@ -28,20 +28,20 @@
 
 void test(void) {}
 
-// RV32-DEFAULT: !{i32 1, !"SmallDataLimit", i32 8}
-// RV32-G4:  !{i32 1, !"SmallDataLimit", i32 4}
-// RV32-S0:  !{i32 1, !"SmallDataLimit", i32 0}
-// RV32-S2G4:!{i32 1, !"SmallDataLimit", i32 4}
-// RV32-T16: !{i32 1, !"SmallDataLimit", i32 16}
-// RV32-PIC: !{i32 1, !"SmallDataLimit", i32 0}
+// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV32-G4:  !{i32 8, !"SmallDataLimit", i32 4}
+// RV32-S0:  !{i32 8, !"SmallDataLimit", i32 0}
+// RV32-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
+// RV32-T16: !{i32 8, !"SmallDataLimit", i32 16}
+// RV32-PIC: !{i32 8, !"SmallDataLimit", i32 0}
 
-// RV64-DEFAULT: !{i32 1, !"SmallDataLimit", i32 8}
-// RV64-G4:  !{i32 1, !"SmallDataLimit", i32 4}
-// RV64-S0:  !{i32 1, !"SmallDataLimit", i32 0}
-// RV64-S2G4:!{i32 1, !"SmallDataLimit", i32 4}
-// RV64-T16: !{i32 1, !"SmallDataLimit", i32 16}
-// RV64-PIC: !{i32 1, !"SmallDataLimit", i32 0}
-// RV64-LARGE:   !{i32 1, !"SmallDataLimit", i32 0}
+// RV64-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV64-G4:  !{i32 8, !"SmallDataLimit", i32 4}
+// RV64-S0:  !{i32 8, !"SmallDataLimit", i32 0}
+// RV64-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
+// RV64-T16: !{i32 8, !"SmallDataLimit", i32 16}
+// RV64-PIC: !{i32 8, !"SmallDataLimit", i32 0}
+// RV64-LARGE:   !{i32 8, !"SmallDataLimit", i32 0}
 
 // The value will be passed by module flag instead of target feature.
 // RV32-S0-NOT: +small-data-limit=
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -980,7 +980,7 @@
 void CodeGenModule::EmitBackendOptionsMetadata(
 const CodeGenOptions CodeGenOpts) {
   if (getTriple().isRISCV()) {
-getModule().addModuleFlag(llvm::Module::Error, "SmallDataLimit",
+getModule().addModuleFlag(llvm::Module::Min, "SmallDataLimit",
   CodeGenOpts.SmallDataLimit);
   }
 }


Index: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vlenb.c
@@ -29,11 +29,11 @@
 //.
 // RV32: !0 = !{i32 1, !"wchar_size", i32 4}
 // RV32: !1 = !{i32 1, !"target-abi", !"ilp32d"}
-// RV32: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV32: !2 = !{i32 8, !"SmallDataLimit", i32 0}
 // RV32: !3 = !{!"vlenb"}
 //.
 // RV64: !0 = !{i32 1, !"wchar_size", i32 4}
 // RV64: !1 = !{i32 1, !"target-abi", !"lp64d"}
-// RV64: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV64: !2 = !{i32 8, !"SmallDataLimit", i32 0}
 // RV64: !3 = !{!"vlenb"}
 //.
Index: clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
===
--- clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
+++ clang/test/CodeGen/RISCV/riscv-sdata-module-flag.c
@@ -28,20 +28,20 @@
 
 void test(void) {}
 
-// RV32-DEFAULT: !{i32 1, !"SmallDataLimit", i32 8}
-// RV32-G4:  !{i32 1, !"SmallDataLimit", i32 4}
-// RV32-S0:  !{i32 1, !"SmallDataLimit", i32 0}
-// RV32-S2G4:!{i32 1, !"SmallDataLimit", i32 4}
-// RV32-T16: !{i32 1, !"SmallDataLimit", i32 16}
-// RV32-PIC: !{i32 1, !"SmallDataLimit", i32 0}
+// RV32-DEFAULT: !{i32 8, !"SmallDataLimit", i32 8}
+// RV32-G4:  !{i32 8, !"SmallDataLimit", i32 4}
+// RV32-S0:  !{i32 8, !"SmallDataLimit", i32 0}
+// RV32-S2G4:!{i32 8, !"SmallDataLimit", i32 4}
+// RV32-T16:

[PATCH] D131230: [RISCV] Allow mismatched SmallDataLimit and use Min for conflicting values

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya reopened this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jobnoorman.

reopening to update the failing testcase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131230/new/

https://reviews.llvm.org/D131230

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131230: [RISCV] Allow mismatched SmallDataLimit and use Min for conflicting values

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

thanks for reverting it. i'll take care of the test failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131230/new/

https://reviews.llvm.org/D131230

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130374: [Passes] add a tail-call-elim pass near the end of the opt pipeline

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Thanks for clarifying!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130374/new/

https://reviews.llvm.org/D130374

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130374: [Passes] add a tail-call-elim pass near the end of the opt pipeline

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

In D130374#3675677 , @vdsered wrote:

> Just JFYI :)
> Yes, probably not worth it

that is interesting. do we know why?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130374/new/

https://reviews.llvm.org/D130374

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130374: [Passes] add a tail-call-elim pass near the end of the opt pipeline

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Thanks a lot for fixing this!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130374/new/

https://reviews.llvm.org/D130374

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123640: [NFC] Make comment consistent with allow|ignore list renamings

[Aditya Kumar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f7b14898fe3: [NFC] Make comment consistent with 
allow|ignore list renamings (authored by hiraditya).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123640/new/

https://reviews.llvm.org/D123640

Files:
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -175,7 +175,7 @@
   DiagnoseErrors);
 }
 
-/// Parse -f(no-)?sanitize-(coverage-)?(white|ignore)list argument's values,
+/// Parse -f(no-)?sanitize-(coverage-)?(allow|ignore)list argument's values,
 /// diagnosing any invalid file paths and validating special case list format.
 static void parseSpecialCaseListArg(const Driver ,
 const llvm::opt::ArgList ,
@@ -185,7 +185,7 @@
 unsigned MalformedSCLErrorDiagID,
 bool DiagnoseErrors) {
   for (const auto *Arg : Args) {
-// Match -fsanitize-(coverage-)?(white|ignore)list.
+// Match -fsanitize-(coverage-)?(allow|ignore)list.
 if (Arg->getOption().matches(SCLOptionID)) {
   Arg->claim();
   std::string SCLPath = Arg->getValue();
@@ -788,7 +788,7 @@
   CoverageFeatures |= CoverageFunc;
   }
 
-  // Parse -fsanitize-coverage-(ignore|white)list options if coverage enabled.
+  // Parse -fsanitize-coverage-(allow|ignore)list options if coverage enabled.
   // This also validates special case lists format.
   // Here, OptSpecifier() acts as a never-matching command-line argument.
   // So, there is no way to clear coverage lists but you can append to them.


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -175,7 +175,7 @@
   DiagnoseErrors);
 }
 
-/// Parse -f(no-)?sanitize-(coverage-)?(white|ignore)list argument's values,
+/// Parse -f(no-)?sanitize-(coverage-)?(allow|ignore)list argument's values,
 /// diagnosing any invalid file paths and validating special case list format.
 static void parseSpecialCaseListArg(const Driver ,
 const llvm::opt::ArgList ,
@@ -185,7 +185,7 @@
 unsigned MalformedSCLErrorDiagID,
 bool DiagnoseErrors) {
   for (const auto *Arg : Args) {
-// Match -fsanitize-(coverage-)?(white|ignore)list.
+// Match -fsanitize-(coverage-)?(allow|ignore)list.
 if (Arg->getOption().matches(SCLOptionID)) {
   Arg->claim();
   std::string SCLPath = Arg->getValue();
@@ -788,7 +788,7 @@
   CoverageFeatures |= CoverageFunc;
   }
 
-  // Parse -fsanitize-coverage-(ignore|white)list options if coverage enabled.
+  // Parse -fsanitize-coverage-(allow|ignore)list options if coverage enabled.
   // This also validates special case lists format.
   // Here, OptSpecifier() acts as a never-matching command-line argument.
   // So, there is no way to clear coverage lists but you can append to them.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123640: Make comment consistent with allow|ignore list renamings

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added a reviewer: thakis.
Herald added a project: All.
hiraditya requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123640

Files:
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -175,7 +175,7 @@
   DiagnoseErrors);
 }
 
-/// Parse -f(no-)?sanitize-(coverage-)?(white|ignore)list argument's values,
+/// Parse -f(no-)?sanitize-(coverage-)?(allow|ignore)list argument's values,
 /// diagnosing any invalid file paths and validating special case list format.
 static void parseSpecialCaseListArg(const Driver ,
 const llvm::opt::ArgList ,
@@ -185,7 +185,7 @@
 unsigned MalformedSCLErrorDiagID,
 bool DiagnoseErrors) {
   for (const auto *Arg : Args) {
-// Match -fsanitize-(coverage-)?(white|ignore)list.
+// Match -fsanitize-(coverage-)?(allow|ignore)list.
 if (Arg->getOption().matches(SCLOptionID)) {
   Arg->claim();
   std::string SCLPath = Arg->getValue();
@@ -788,7 +788,7 @@
   CoverageFeatures |= CoverageFunc;
   }
 
-  // Parse -fsanitize-coverage-(ignore|white)list options if coverage enabled.
+  // Parse -fsanitize-coverage-(allow|ignore)list options if coverage enabled.
   // This also validates special case lists format.
   // Here, OptSpecifier() acts as a never-matching command-line argument.
   // So, there is no way to clear coverage lists but you can append to them.


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -175,7 +175,7 @@
   DiagnoseErrors);
 }
 
-/// Parse -f(no-)?sanitize-(coverage-)?(white|ignore)list argument's values,
+/// Parse -f(no-)?sanitize-(coverage-)?(allow|ignore)list argument's values,
 /// diagnosing any invalid file paths and validating special case list format.
 static void parseSpecialCaseListArg(const Driver ,
 const llvm::opt::ArgList ,
@@ -185,7 +185,7 @@
 unsigned MalformedSCLErrorDiagID,
 bool DiagnoseErrors) {
   for (const auto *Arg : Args) {
-// Match -fsanitize-(coverage-)?(white|ignore)list.
+// Match -fsanitize-(coverage-)?(allow|ignore)list.
 if (Arg->getOption().matches(SCLOptionID)) {
   Arg->claim();
   std::string SCLPath = Arg->getValue();
@@ -788,7 +788,7 @@
   CoverageFeatures |= CoverageFunc;
   }
 
-  // Parse -fsanitize-coverage-(ignore|white)list options if coverage enabled.
+  // Parse -fsanitize-coverage-(allow|ignore)list options if coverage enabled.
   // This also validates special case lists format.
   // Here, OptSpecifier() acts as a never-matching command-line argument.
   // So, there is no way to clear coverage lists but you can append to them.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file,function}-list= to match gcc options.

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.
Herald added a project: All.

Is there a plan to revive this? this is quite necessary for one of my use cases.
can we at least have '-fno-instrument-functions'?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D37624/new/

https://reviews.llvm.org/D37624

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f00aa502d79: Add no_instrument_function attribute to 
Objective C methods as well (authored by hiraditya).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111286/new/

https://reviews.llvm.org/D111286

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/instrument-objc-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-noinstrument.m


Index: clang/test/SemaObjC/attr-noinstrument.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-noinstrument.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10.4 -verify 
-Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -triple 
x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+@interface A
++ (void)F __attribute__((no_instrument_function)); // no warning
+- (void)f __attribute__((objc_direct, no_instrument_function));
+- (void)g;
+@end
+
+@implementation A
++ (void)F __attribute__((no_instrument_function)) {
+  [self F];
+}
+
+- (void)f {
+  [self g];
+}
+
+- (void)g {
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -98,7 +98,7 @@
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NoInline (SubjectMatchRule_function)
-// CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function)
+// CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | 
FileCheck %s
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare 
| FileCheck -check-prefix=BARE %s
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize {
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function)) {
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+- (void)dealloc __attribute__((no_instrument_function)) {
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }


Index: clang/test/SemaObjC/attr-noinstrument.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-noinstrument.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -triple x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+@interface A
++ (void)F __attribute__((no_instrument_function)); // no warning
+- (void)f __attribute__((objc_direct, no_instrument_function));
+- (void)g;
+@end
+
+@implementation A
++ (void)F __attribute__((no_instrument_function)) {
+  [self F];
+}
+
+- (void)f {
+  [self g];
+}
+
+- (void)g {
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ 

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 377934.
hiraditya added a comment.

clang-format


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111286/new/

https://reviews.llvm.org/D111286

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/instrument-objc-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-noinstrument.m


Index: clang/test/SemaObjC/attr-noinstrument.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-noinstrument.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10.4 -verify 
-Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -triple 
x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+@interface A
++ (void)F __attribute__((no_instrument_function)); // no warning
+- (void)f __attribute__((objc_direct, no_instrument_function));
+- (void)g;
+@end
+
+@implementation A
++ (void)F __attribute__((no_instrument_function)) {
+  [self F];
+}
+
+- (void)f {
+  [self g];
+}
+
+- (void)g {
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -98,7 +98,7 @@
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NoInline (SubjectMatchRule_function)
-// CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function)
+// CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | 
FileCheck %s
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare 
| FileCheck -check-prefix=BARE %s
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize {
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function)) {
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+- (void)dealloc __attribute__((no_instrument_function)) {
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }


Index: clang/test/SemaObjC/attr-noinstrument.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-noinstrument.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -triple x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+@interface A
++ (void)F __attribute__((no_instrument_function)); // no warning
+- (void)f __attribute__((objc_direct, no_instrument_function));
+- (void)g;
+@end
+
+@implementation A
++ (void)F __attribute__((no_instrument_function)) {
+  [self F];
+}
+
+- (void)f {
+  [self g];
+}
+
+- (void)g {
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -98,7 +98,7 @@
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoEscape 

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 377895.
hiraditya added a comment.
Herald added a subscriber: jdoerfert.

Added SemaObjc test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111286/new/

https://reviews.llvm.org/D111286

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/instrument-objc-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-noinstrument.m


Index: clang/test/SemaObjC/attr-noinstrument.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-noinstrument.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10.4 -verify 
-Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -triple 
x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+@interface A
++ (void)F __attribute__((no_instrument_function)); // no warning
+- (void)f __attribute__((objc_direct, no_instrument_function));
+- (void)g;
+@end
+
+@implementation A
++ (void)F __attribute__((no_instrument_function))
+{
+  [self F];
+}
+
+- (void)f
+{
+  [self g];
+}
+
+- (void)g
+{
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -98,7 +98,7 @@
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NoInline (SubjectMatchRule_function)
-// CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function)
+// CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | 
FileCheck %s
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare 
| FileCheck -check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize
+{
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }


Index: clang/test/SemaObjC/attr-noinstrument.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-noinstrument.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -triple x86_64-apple-darwin10.4 -verify -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+@interface A
++ (void)F __attribute__((no_instrument_function)); // no warning
+- (void)f __attribute__((objc_direct, no_instrument_function));
+- (void)g;
+@end
+
+@implementation A
++ (void)F __attribute__((no_instrument_function))
+{
+  [self F];
+}
+
+- (void)f
+{
+  [self g];
+}
+
+- (void)g
+{
+}
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -98,7 +98,7 @@
 // CHECK-NEXT: NoDuplicate 

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 377752.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111286/new/

https://reviews.llvm.org/D111286

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/instrument-objc-method.m


Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | 
FileCheck %s
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare 
| FileCheck -check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize
+{
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }


Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare | FileCheck -check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize
+{
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 377751.
hiraditya added a comment.

Testcase ready.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111286/new/

https://reviews.llvm.org/D111286

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/instrument-objc-method.m


Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | 
FileCheck %s
+// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s 
-finstrument-function-entry-bare | FileCheck -check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize
+{
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }


Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s
+// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare | FileCheck -check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @"\01+[ObjCClass initialize]"
+// CHECK: call void @__cyg_profile_func_enter
+// CHECK: call void @__cyg_profile_func_exit
+// BARE: @"\01+[ObjCClass initialize]"
+// BARE: call void @__cyg_profile_func_enter
++ (void)initialize
+{
+}
+
+// CHECK: @"\01+[ObjCClass load]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01+[ObjCClass load]"
+// BARE-NOT: call void @__cyg_profile_func_enter
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: @"\01-[ObjCClass dealloc]"
+// CHECK-NOT: call void @__cyg_profile_func_enter
+// BARE: @"\01-[ObjCClass dealloc]"
+// BARE-NOT: call void @__cyg_profile_func_enter
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+
+// CHECK: declare void @__cyg_profile_func_enter(i8*, i8*)
+// CHECK: declare void @__cyg_profile_func_exit(i8*, i8*)
+// BARE: declare void @__cyg_profile_func_enter_bare
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Still working on adding the testcase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111286/new/

https://reviews.llvm.org/D111286

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111286: Add no_instrument_function attribute to Objective C methods as well

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added a reviewer: rjmccall.
Herald added a reviewer: aaron.ballman.
hiraditya requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There are several methods where we do not want function instrumentation which 
is why we have `__attribute__((no_instrument_function))`. Extending this 
functionality to disable instrumentation for Objective-C methods as well. 
Objective C methods like `+load` run premain and having instrumentation on them 
causes runtime errors depending on the implementation of 
`__cyg_profile_func_enter` etc. functions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111286

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/instrument-objc-method.m


Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,24 @@
+#import 
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 
-debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions 
-disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s 
-finstrument-function-entry-bare -disable-llvm-passes | FileCheck 
-check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @load
+// CHECK: "instrument-function-entry"="__cyg_profile_func_enter"
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
++ (void)initialize
+{
+}
+
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }


Index: clang/test/CodeGen/instrument-objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/instrument-objc-method.m
@@ -0,0 +1,24 @@
+#import 
+// RUN: %clang_cc1 -S -triple x86_64-apple-darwin10 -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare -disable-llvm-passes | FileCheck -check-prefix=BARE %s
+
+
+@interface ObjCClass
+@end
+
+@implementation ObjCClass
+
+// CHECK: @load
+// CHECK: "instrument-function-entry"="__cyg_profile_func_enter"
++ (void)load __attribute__((no_instrument_function))
+{
+}
+
++ (void)initialize
+{
+}
+
+-(void)dealloc __attribute__((no_instrument_function))
+{
+}
+@end
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1979,7 +1979,7 @@
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96685: [WIP] Add noexcept clang-tidy codemod

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96685

Files:
  clang-tools-extra/clang-tidy/modernize/AddNoexceptCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AddNoexceptCheck.h

Index: clang-tools-extra/clang-tidy/modernize/AddNoexceptCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/AddNoexceptCheck.h
@@ -0,0 +1,39 @@
+//===--- AddNoexceptCheck.h - clang-tidy-*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_ADD_NOEXCEPT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_ADD_NOEXCEPT_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Add noexcept to function delcaration and definitions. 
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-add-noexcept.html
+class AddNoexceptCheck : public ClangTidyCheck {
+public:
+  AddNoexceptCheck(StringRef Name, ClangTidyContext *Context);
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus11;
+  }
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  const std::string NoexceptMacro;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_ADD_NOEXCEPT_H
Index: clang-tools-extra/clang-tidy/modernize/AddNoexceptCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/AddNoexceptCheck.cpp
@@ -0,0 +1,121 @@
+//===--- AddNoexceptCheck.cpp - clang-tidy-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// TODOs:
+// - Handle const functions properly.
+// - Handle functions with attributes.
+// - Handle functions with weird qualifiers (&, &&). There may not be too many 
+//  instances of this.
+//===--===//
+
+#include "AddNoexceptCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+namespace {
+AST_MATCHER(NamedDecl, isValid) { return !Node.isInvalidDecl(); }
+AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
+AST_MATCHER(CXXMethodDecl, hasRefQualifier) { return Node.getRefQualifier() != clang::RQ_None; }
+AST_MATCHER(CXXMethodDecl, hasTrivialBody) { return Node.hasTrivialBody(); }
+
+AST_MATCHER(CXXRecordDecl, hasAnyDependentBases) {
+  return Node.hasAnyDependentBases();
+}
+
+AST_MATCHER(CXXMethodDecl, isTemplate) {
+  return Node.getTemplatedKind() != FunctionDecl::TK_NonTemplate;
+}
+
+AST_MATCHER(CXXMethodDecl, isDependentContext) {
+  return Node.isDependentContext();
+}
+
+AST_MATCHER(CXXMethodDecl, isInsideMacroDefinition) {
+  const ASTContext  = Finder->getASTContext();
+  return clang::Lexer::makeFileCharRange(
+ clang::CharSourceRange::getCharRange(
+ Node.getTypeSourceInfo()->getTypeLoc().getSourceRange()),
+ Ctxt.getSourceManager(), Ctxt.getLangOpts())
+  .isInvalid();
+}
+
+// hasDynamicExceptionSpec mathes all 'throw's in the qualifier. We also skip all 'except's in the qualifier.
+AST_MATCHER(CXXMethodDecl, hasExceptSpec) { return clang::isNoexceptExceptionSpec(Node.getExceptionSpecType()); }
+
+AST_MATCHER_P(CXXMethodDecl, hasCanonicalDecl,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  return InnerMatcher.matches(*Node.getCanonicalDecl(), Finder, Builder);
+}
+
+} // namespace
+
+static SourceLocation getConstInsertionPoint(const CXXMethodDecl *M) {
+  TypeSourceInfo *TSI = M->getTypeSourceInfo();
+  if (!TSI)
+return {};
+
+  FunctionTypeLoc FTL = TSI->getTypeLoc().IgnoreParens().getAs();
+  if (!FTL)
+return {};
+  if (M->isConst()) {
+// TODO: Use a while loop to find the end of const.
+return FTL.getRParenLoc().getLocWithOffset(7);
+  }
+  return FTL.getRParenLoc().getLocWithOffset(1);
+}
+
+AddNoexceptCheck::AddNoexceptCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

cc: @t.p.northover does this fix look reasonable for PR37240?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83788: Removed unused variable in clang

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Not sure why this didnt get closed after the patch is merged in: 
https://reviews.llvm.org/rG4fd91b0f946f49370a3ab33c480a807a3f48b247


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83788/new/

https://reviews.llvm.org/D83788

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83788: Removed unused variable in clang

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83788/new/

https://reviews.llvm.org/D83788



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.
Herald added a subscriber: danielkiss.

do we have a plan to follow up on this patch?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82832: Correctly generate invert xor value for Binary Atomics of int size > 64

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Nice find!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82832/new/

https://reviews.llvm.org/D82832



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:801
+if (Branch->isUnconditional())
+  if (ReturnInst *Ret = dyn_cast(
+  Branch->getSuccessor(0)->getFirstNonPHIOrDbg()))

can we use isa<> here?



Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:816
+if (CallInst *CI = dyn_cast(&*BBI))
+  if (!canMoveAboveCall(CI, Inst, AA) && CI->getCalledFunction() != )
+return false;

`CI->getCalledFunction() != ` seems cheaper than `canMoveAboveCall`



Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:840
+
+  return !llvm::any_of(instructions(F), [&](Instruction ) {
+// Because of PR962, we don't TRE dynamic allocas.

Do we need to visit all the instructions twice?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82085/new/

https://reviews.llvm.org/D82085



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: libcxx/trunk/include/__split_buffer:201
 __alloc_rr& __a = this->__alloc();
+pointer __to_be_end = this->__end_;
 do

danlark wrote:
> lichray wrote:
> > mclow.lists wrote:
> > > I have been asked specifically by the optimizer folks to NOT do things 
> > > like this in libc++, but rather to file bugs against the optimizer.
> > > 
> > > And I have done so for this exact case:  
> > > https://bugs.llvm.org/show_bug.cgi?id=35637
> > From the thread I didn't see that the compiler side asked you not to do so.
> > 
> > And I disagree with the view.  libc++ shouldn't *wait* for compilers, 
> > because we don't dictate users' compiler choices.  This change doesn't make 
> > libc++ worse to coming compilers, and makes libc++ better on existing 
> > compilers, so what benefit we get by not approving this?
> So, what is the status? Are we waiting for the compiler code-gen fix?
> 
> At Yandex we are using patched version like half a year or more.
> 
> https://github.com/catboost/catboost/blob/master/contrib/libs/cxxsupp/libcxx/include/vector#L995
It would be great to get this patch in. Waiting for compiler for this 
optimization seems overkill.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44823/new/

https://reviews.llvm.org/D44823



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72873: [clang][xray] Add -fxray-ignore-loops option

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.

The test case is in another patch so LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72873/new/

https://reviews.llvm.org/D72873



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72873: [clang][xray] Add -fxray-ignore-loops option

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:821
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));
+  if (CGM.getCodeGenOpts().XRayIgnoreLoops) {
+Fn->addFnAttr("xray-ignore-loops");

we can remove braces here.



Comment at: clang/test/CodeGen/xray-ignore-loops.cpp:5
+  return 1;
+}
+

I think we need one more test case of a function having a loop.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72873/new/

https://reviews.llvm.org/D72873



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22834: Added 'inline' attribute to basic_string's destructor

[Aditya Kumar via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb839888af8f2: Added inline attribute to 
basic_strings destructor (authored by hiraditya).
Herald added subscribers: libcxx-commits, christof.
Herald added a project: libc++.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D22834/new/

https://reviews.llvm.org/D22834

Files:
  libcxx/include/string


Index: libcxx/include/string
===
--- libcxx/include/string
+++ libcxx/include/string
@@ -1798,6 +1798,7 @@
 #endif  // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>::~basic_string()
 {
 #if _LIBCPP_DEBUG_LEVEL >= 2


Index: libcxx/include/string
===
--- libcxx/include/string
+++ libcxx/include/string
@@ -1798,6 +1798,7 @@
 #endif  // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>::~basic_string()
 {
 #if _LIBCPP_DEBUG_LEVEL >= 2
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Can we add a test to verify cfi instructions are present without debug flag.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya closed this revision.
hiraditya added a comment.
Herald added a project: clang.

closed in r345828, 314fbfa1c4c6665c54a220eefb10a6f23010a352 



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53514/new/

https://reviews.llvm.org/D53514



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

it will be great to merge this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57265/new/

https://reviews.llvm.org/D57265



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Thanks for addressing the feedback @emmettneyman , LGTM, deferring to other 
reviewers for final call.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64380/new/

https://reviews.llvm.org/D64380



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57265/new/

https://reviews.llvm.org/D57265



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50737: [ASTImporter] Add test for CXXNoexceptExpr

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: test/Import/cxx-noexcept-expr/test.cpp:1
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+

Can we add a line/comment to explain what this test does? Same for the input 
files.


Repository:
  rC Clang

https://reviews.llvm.org/D50737



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50796: [ASTImporter] Add test for IfStmt

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the test.


Repository:
  rC Clang

https://reviews.llvm.org/D50796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50766: Fix false positive unsequenced access and modification warning in array subscript expression.

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:11678
 // Forget that LHS and RHS are sequenced. They are both unsequenced
 // with respect to other stuff.
+Tree.merge(LHSRegion);

Is this comment still relevant?


Repository:
  rC Clang

https://reviews.llvm.org/D50766



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50674: [libc++] Add missing #include in C11 features tests

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya accepted this revision.
hiraditya added a comment.
This revision is now accepted and ready to land.

Good catch!


Repository:
  rCXX libc++

https://reviews.llvm.org/D50674



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D50699#1198770, @owenpan wrote:

> Test case:
>  F6937162: BreakBeforeBinaryOperators.cpp 


Please add a testcase in the source and add checks to verify the changes.


Repository:
  rC Clang

https://reviews.llvm.org/D50699



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47987: Provide only one declaration of __throw_runtime_error

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.
Herald added a subscriber: ldionne.

Any updates on this one? I like @mclow.lists idea of removing it from __locale.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47987



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45935: Add dump method for selectors

[Aditya Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333657: Add dump method for selectors (authored by 
hiraditya, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45935?vs=143493=149294#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45935

Files:
  cfe/trunk/include/clang/Basic/IdentifierTable.h
  cfe/trunk/lib/Basic/IdentifierTable.cpp


Index: cfe/trunk/include/clang/Basic/IdentifierTable.h
===
--- cfe/trunk/include/clang/Basic/IdentifierTable.h
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h
@@ -763,6 +763,8 @@
   /// Prints the full selector name (e.g. "foo:bar:").
   void print(llvm::raw_ostream ) const;
 
+  void dump() const;
+
   /// Derive the conventional family of this method.
   ObjCMethodFamily getMethodFamily() const {
 return getMethodFamilyImpl(*this);
Index: cfe/trunk/lib/Basic/IdentifierTable.cpp
===
--- cfe/trunk/lib/Basic/IdentifierTable.cpp
+++ cfe/trunk/lib/Basic/IdentifierTable.cpp
@@ -504,6 +504,8 @@
   OS << getAsString();
 }
 
+LLVM_DUMP_METHOD void Selector::dump() const { print(llvm::errs()); }
+
 /// Interpreting the given string using the normal CamelCase
 /// conventions, determine whether the given string starts with the
 /// given "word", which is assumed to end in a lowercase letter.


Index: cfe/trunk/include/clang/Basic/IdentifierTable.h
===
--- cfe/trunk/include/clang/Basic/IdentifierTable.h
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h
@@ -763,6 +763,8 @@
   /// Prints the full selector name (e.g. "foo:bar:").
   void print(llvm::raw_ostream ) const;
 
+  void dump() const;
+
   /// Derive the conventional family of this method.
   ObjCMethodFamily getMethodFamily() const {
 return getMethodFamilyImpl(*this);
Index: cfe/trunk/lib/Basic/IdentifierTable.cpp
===
--- cfe/trunk/lib/Basic/IdentifierTable.cpp
+++ cfe/trunk/lib/Basic/IdentifierTable.cpp
@@ -504,6 +504,8 @@
   OS << getAsString();
 }
 
+LLVM_DUMP_METHOD void Selector::dump() const { print(llvm::errs()); }
+
 /// Interpreting the given string using the normal CamelCase
 /// conventions, determine whether the given string starts with the
 /// given "word", which is assumed to end in a lowercase letter.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30035: Add const to function parameters

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya abandoned this revision.
hiraditya added a comment.

Merged with https://reviews.llvm.org/D30268


https://reviews.llvm.org/D30035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45601: Warn on bool* to bool conversion

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya abandoned this revision.
hiraditya added a comment.

It appears this warning may not be always useful because there will be false 
positives.


https://reviews.llvm.org/D45601



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45601: Warn on bool* to bool conversion

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

The bug which motivated this warning is: 
https://github.com/jemalloc/jemalloc/commit/4df483f0fd76a64e116b1c4f316f8b941078114d#diff-7b26b977303fe92c093a2245b0eaf255


https://reviews.llvm.org/D45601



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45601: Warn on bool* to bool conversion

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 142776.
hiraditya added a comment.

Warn on bool* to bool conversion during a call only.


https://reviews.llvm.org/D45601

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-bool-ptr-to-bool.cpp

Index: test/SemaCXX/warn-bool-ptr-to-bool.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-bool-ptr-to-bool.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(bool *b) {
+  if (b) // no-warning
+return 10;
+  return 0;
+}
+
+int bar(bool b) {
+  return b;
+}
+
+int baz() {
+  bool *b;
+  bar(b);
+  // expected-warning@-1 {{passing 'bool *' as a boolean}}
+  return 0;
+}
+
+template
+T foo1(T *ptr) {
+  return ptr ? *ptr : T{}; // no-warning
+}
+
+bool bar1(bool *ptr) {
+  return foo1(ptr);
+}
+
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2545,6 +2545,27 @@
   }
 }
 
+/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
+void DiagnoseImpCast(Sema , const Expr *E, QualType SourceType, QualType T,
+ SourceLocation CContext, unsigned diag,
+ bool pruneControlFlow = false) {
+  if (pruneControlFlow) {
+S.DiagRuntimeBehavior(E->getExprLoc(), E,
+  S.PDiag(diag)
+<< SourceType << T << E->getSourceRange()
+<< SourceRange(CContext));
+return;
+  }
+  S.Diag(E->getExprLoc(), diag)
+<< SourceType << T << E->getSourceRange() << SourceRange(CContext);
+}
+
+/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
+void DiagnoseImpCast(Sema , const Expr *E, QualType T, SourceLocation CContext,
+ unsigned diag, bool pruneControlFlow = false) {
+  DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
+}
+
 /// Handles the checks for format strings, non-POD arguments to vararg
 /// functions, NULL arguments passed to non-NULL parameters, and diagnose_if
 /// attributes.
@@ -2589,6 +2610,28 @@
 }
   }
 
+  if (FD) {
+for (unsigned ArgIdx = 0; ArgIdx < Args.size(); ++ArgIdx) {
+  // Args[ArgIdx] can be null in malformed code.
+  if (const Expr *Arg = Args[ArgIdx]) {
+if (auto C = dyn_cast(Arg)) {
+  if (C->getCastKind() == CK_PointerToBoolean) {
+if (auto ICast = dyn_cast(C->getSubExpr())) {
+  if (ICast->getCastKind() == CK_LValueToRValue) {
+const Expr *Pointer = ICast->getSubExpr();
+QualType QT = Pointer->getType()->getPointeeType();
+if (!QT.isNull() && QT->isBooleanType())
+  // Warn on bool* to bool conversion.
+  DiagnoseImpCast(*this, Pointer, QT, Arg->getLocStart(),
+  diag::warn_pointer_to_bool);
+  }
+}
+  }
+}
+  }
+}
+  }
+
   if (FDecl || Proto) {
 CheckNonNullArguments(*this, FDecl, Proto, Args, Loc);
 
@@ -8941,27 +8984,6 @@
   AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
 }
 
-/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
-void DiagnoseImpCast(Sema , Expr *E, QualType SourceType, QualType T, 
- SourceLocation CContext, unsigned diag,
- bool pruneControlFlow = false) {
-  if (pruneControlFlow) {
-S.DiagRuntimeBehavior(E->getExprLoc(), E,
-  S.PDiag(diag)
-<< SourceType << T << E->getSourceRange()
-<< SourceRange(CContext));
-return;
-  }
-  S.Diag(E->getExprLoc(), diag)
-<< SourceType << T << E->getSourceRange() << SourceRange(CContext);
-}
-
-/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
-void DiagnoseImpCast(Sema , Expr *E, QualType T, SourceLocation CContext,
- unsigned diag, bool pruneControlFlow = false) {
-  DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
-}
-
 
 /// Diagnose an implicit cast from a floating point value to an integer value.
 void DiagnoseFloatingImpCast(Sema , Expr *E, QualType T,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7760,6 +7760,9 @@
   "format specifies type %0 but the argument has "
   "%select{type|underlying type}2 %1">,
   InGroup;
+def warn_pointer_to_bool : Warning<
+  "passing %0 as a boolean">,
+  InGroup;
 def warn_format_argument_needs_cast : Warning<
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,

[PATCH] D45601: Warn on bool* to bool conversion

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

I'll probably make this warning for function arguments only (when bool* is 
passed to a function accepting bool). Many conditionals use bool* -> bool 
conversion as pointed out by  @Quuxplusone and @aaron.ballman


https://reviews.llvm.org/D45601



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45601: Warn on bool* to bool conversion

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: Eugene.Zelenko, rsmith.

We found conversion from bool* to bool to be a common source of bug, so we have 
implemented this warning. Sample use case:

  int bar(bool b) {
return b;
  }
  
  int baz() {
bool *b; 
bar(b);
return 0;
  }

Typically, there would be a function which takes a bool, which gets a pointer 
to boolean at the call site. The compiler currently does not warn which results
in a difficult to debug runtime failure.


https://reviews.llvm.org/D45601

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
  test/Sema/static-init.c
  test/SemaCXX/warn-bool-ptr-to-bool.cpp


Index: test/SemaCXX/warn-bool-ptr-to-bool.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-bool-ptr-to-bool.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(bool *b) {
+  if (b)
+// expected-warning@-1 {{comparing 'bool *' as a boolean}}
+return 10;
+  return 0;
+}
+
+
+int bar(bool b) {
+  return b;
+}
+
+int baz() {
+  bool *b;
+  bar(b);
+  // expected-warning@-1 {{comparing 'bool *' as a boolean}}
+  return 0;
+}
+
+
+
Index: test/Sema/static-init.c
===
--- test/Sema/static-init.c
+++ test/Sema/static-init.c
@@ -7,7 +7,7 @@
 
 float r  = (float) (intptr_t)  // expected-error {{initializer element is 
not a compile-time constant}}
 intptr_t s = (intptr_t) 
-_Bool t = 
+_Bool t =  // expected-warning {{comparing '_Bool *' as a boolean}}
 
 
 union bar {
Index: test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
===
--- test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
+++ test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
@@ -16,6 +16,7 @@
 
 // --  pointer, 
 bool b6 = ! // expected-warning{{address of 'b4' will always evaluate to 
'true'}}
+// expected-warning@-1 {{comparing 'bool *' as a boolean}}
 void f();
 bool b61 = !
 
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9610,6 +9610,12 @@
   S.DiagnoseAlwaysNonNullPointer(E, Expr::NPCK_NotNull, /*IsEqual*/ false,
  SourceRange(CC));
 }
+if (Source->isPointerType() || Source->canDecayToPointerType()) {
+  QualType PointeeType = Source->getPointeeType();
+  // Warn on bool* to bool conversion.
+  if (!PointeeType.isNull() && PointeeType->isBooleanType())
+return DiagnoseImpCast(S, E, T, CC, diag::warn_pointer_to_bool);
+}
   }
 
   // Check implicit casts from Objective-C collection literals to specialized
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7801,6 +7801,9 @@
   "format specifies type %0 but the argument has "
   "%select{type|underlying type}2 %1">,
   InGroup;
+def warn_pointer_to_bool : Warning<
+  "comparing %0 as a boolean">,
+  InGroup;
 def warn_format_argument_needs_cast : Warning<
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,


Index: test/SemaCXX/warn-bool-ptr-to-bool.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-bool-ptr-to-bool.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(bool *b) {
+  if (b)
+// expected-warning@-1 {{comparing 'bool *' as a boolean}}
+return 10;
+  return 0;
+}
+
+
+int bar(bool b) {
+  return b;
+}
+
+int baz() {
+  bool *b;
+  bar(b);
+  // expected-warning@-1 {{comparing 'bool *' as a boolean}}
+  return 0;
+}
+
+
+
Index: test/Sema/static-init.c
===
--- test/Sema/static-init.c
+++ test/Sema/static-init.c
@@ -7,7 +7,7 @@
 
 float r  = (float) (intptr_t)  // expected-error {{initializer element is not a compile-time constant}}
 intptr_t s = (intptr_t) 
-_Bool t = 
+_Bool t =  // expected-warning {{comparing '_Bool *' as a boolean}}
 
 
 union bar {
Index: test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
===
--- test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
+++ test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
@@ -16,6 +16,7 @@
 
 // --  pointer, 
 bool b6 = ! // expected-warning{{address of 'b4' will always evaluate to 'true'}}
+// expected-warning@-1 {{comparing 'bool *' as a boolean}}
 void f();
 bool b61 = !
 
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9610,6 +9610,12 @@
   

[PATCH] D36423: [libc++] Introsort based sorting function

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Ping!


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34224: [NFC] remove trailing WS

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya closed this revision.
hiraditya added a comment.

Closed by commit: https://reviews.llvm.org/rL311283


https://reviews.llvm.org/D34224



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36423: [libc++] Introsort based sorting function

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Results with the patch.

Before:

  Run on (8 X 3900 MHz CPU s)
  2017-08-20 15:11:41
  
---
  BenchmarkTime   CPU Iterations
  
---
  BM_Sort/random_uint32/65536   14202353 ns   14203202 ns 48
  BM_Sort/sorted_ascending_uint32/65536   254100 ns 254108 ns   2754
  BM_Sort/sorted_descending_uint32/65536  552118 ns 552151 ns   1232
  BM_Sort/single_element_uint32/65536 170140 ns 170136 ns   4090
  BM_Sort/pipe_organ_uint32/655365989117 ns5989494 ns113
  BM_Sort/random_strings/65536 105697682 ns  105702553 ns  7
  BM_Sort/sorted_ascending_strings/6553613324109 ns   13324186 ns 50
  BM_Sort/sorted_descending_strings/65536   19057303 ns   19058005 ns 36
  BM_Sort/single_element_strings/65536  57941433 ns   57944691 ns 12
  BM_Sort/qsort_worst_uint32/65536 694858550 ns  694894213 ns  1

After:

  Run on (8 X 3900 MHz CPU s)
  2017-08-20 15:15:14
  
---
  BenchmarkTime   CPU Iterations
  
---
  BM_Sort/random_uint32/65536   14073209 ns   14073732 ns 49
  BM_Sort/sorted_ascending_uint32/65536   257596 ns 257610 ns   2740
  BM_Sort/sorted_descending_uint32/65536  560208 ns 560069 ns   1226
  BM_Sort/single_element_uint32/65536 170543 ns 170549 ns   4075
  BM_Sort/pipe_organ_uint32/655366008832 ns6009173 ns113
  BM_Sort/random_strings/65536 104672888 ns  104677220 ns  7
  BM_Sort/sorted_ascending_strings/6553613334016 ns   13334393 ns 54
  BM_Sort/sorted_descending_strings/65536   18883275 ns   18883831 ns 37
  BM_Sort/single_element_strings/65536  57022905 ns   57025206 ns 12
  BM_Sort/qsort_worst_uint32/65536  16870788 ns   16871828 ns 41


https://reviews.llvm.org/D36423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34224: [NFC] remove trailing WS

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.

https://reviews.llvm.org/D34224

Files:
  include/memory

Index: include/memory
===
--- include/memory
+++ include/memory
@@ -724,7 +724,7 @@
 struct __has_element_type : false_type {};
 
 template 
-struct __has_element_type<_Tp, 
+struct __has_element_type<_Tp,
   typename __void_t::type> : true_type {};
 
 template ::value>
@@ -808,7 +808,7 @@
 struct __has_difference_type : false_type {};
 
 template 
-struct __has_difference_type<_Tp, 
+struct __has_difference_type<_Tp,
 typename __void_t::type> : true_type {};
 
 template ::value>
@@ -994,7 +994,7 @@
 struct __has_pointer_type : false_type {};
 
 template 
-struct __has_pointer_type<_Tp, 
+struct __has_pointer_type<_Tp,
   typename __void_t::type> : true_type {};
 
 namespace __pointer_type_imp
@@ -1024,7 +1024,7 @@
 struct __has_const_pointer : false_type {};
 
 template 
-struct __has_const_pointer<_Tp, 
+struct __has_const_pointer<_Tp,
 typename __void_t::type> : true_type {};
 
 template ::value>
@@ -1047,7 +1047,7 @@
 struct __has_void_pointer : false_type {};
 
 template 
-struct __has_void_pointer<_Tp, 
+struct __has_void_pointer<_Tp,
typename __void_t::type> : true_type {};
 
 template ::value>
@@ -1070,7 +1070,7 @@
 struct __has_const_void_pointer : false_type {};
 
 template 
-struct __has_const_void_pointer<_Tp, 
+struct __has_const_void_pointer<_Tp,
 typename __void_t::type> : true_type {};
 
 template ::value>
@@ -1148,7 +1148,7 @@
 struct __has_propagate_on_container_move_assignment : false_type {};
 
 template 
-struct __has_propagate_on_container_move_assignment<_Tp, 
+struct __has_propagate_on_container_move_assignment<_Tp,
typename __void_t::type>
: true_type {};
 
@@ -1168,7 +1168,7 @@
 struct __has_propagate_on_container_swap : false_type {};
 
 template 
-struct __has_propagate_on_container_swap<_Tp, 
+struct __has_propagate_on_container_swap<_Tp,
typename __void_t::type>
: true_type {};
 
@@ -1188,7 +1188,7 @@
 struct __has_is_always_equal : false_type {};
 
 template 
-struct __has_is_always_equal<_Tp, 
+struct __has_is_always_equal<_Tp,
typename __void_t::type>
: true_type {};
 
@@ -1941,7 +1941,7 @@
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator  operator++(int)
 {raw_storage_iterator __t(*this); ++__x_; return __t;}
 #if _LIBCPP_STD_VER >= 14
-_LIBCPP_INLINE_VISIBILITY _OutputIterator base() const { return __x_; } 
+_LIBCPP_INLINE_VISIBILITY _OutputIterator base() const { return __x_; }
 #endif
 };
 
@@ -3850,7 +3850,7 @@
 _LIBCPP_INLINE_VISIBILITY
 _Dp* __get_deleter() const _NOEXCEPT
 {return static_cast<_Dp*>(__cntrl_
-? const_cast(__cntrl_->__get_deleter(typeid(_Dp))) 
+? const_cast(__cntrl_->__get_deleter(typeid(_Dp)))
   : nullptr);}
 #endif  // _LIBCPP_NO_RTTI
 
@@ -4477,7 +4477,7 @@
 typename enable_if
 <
 !is_array<_Yp>::value &&
-is_convertible::pointer, 
+is_convertible::pointer,
typename shared_ptr<_Tp>::element_type*>::value,
 shared_ptr<_Tp>&
 >::type
@@ -4512,7 +4512,7 @@
 typename enable_if
 <
 !is_array<_Yp>::value &&
-is_convertible::pointer, 
+is_convertible::pointer,
typename shared_ptr<_Tp>::element_type*>::value,
 shared_ptr<_Tp>&
 >::type
@@ -5311,7 +5311,7 @@
 __m.unlock();
 return __q;
 }
-  
+
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 _LIBCPP_AVAILABILITY_ATOMIC_SHARED_PTR
@@ -5352,7 +5352,7 @@
 __m.unlock();
 return __r;
 }
-  
+
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 _LIBCPP_AVAILABILITY_ATOMIC_SHARED_PTR
@@ -5484,7 +5484,7 @@
 _NOEXCEPT_(__is_nothrow_swappable<_Alloc>::value)
 #endif
 {
-__swap_allocator(__a1, __a2, 
+__swap_allocator(__a1, __a2,
   integral_constant::propagate_on_container_swap::value>());
 }
 
@@ -5506,7 +5506,7 @@
 void __swap_allocator(_Alloc &, _Alloc &, false_type) _NOEXCEPT {}
 
 template  >
-struct __noexcept_move_assign_container : public integral_constant 14
 || _Traits::is_always_equal::value
@@ -5520,17 +5520,17 @@
 template 
 struct __temp_value {
 typedef allocator_traits<_Alloc> _Traits;
-
+
 typename aligned_storage::type __v;
 _Alloc &__a;
 
 _Tp *__addr() { return reinterpret_cast<_Tp *>(addressof(__v)); }
 _Tp &   get() { return *__addr(); }
-
+
 template
 __temp_value(_Alloc &__alloc, _Args&& ... __args) : __a(__alloc)
 { _Traits::construct(__a, __addr(), _VSTD::forward<_Args>(__args)...); }
-
+
 

[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Aditya Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305427: [locale] Avoid copy of __atoms when char_type is 
char (authored by hiraditya).

Changed prior to commit:
  https://reviews.llvm.org/D30268?vs=102566=102621#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30268

Files:
  libcxx/trunk/benchmarks/stringstream.bench.cpp
  libcxx/trunk/include/__config
  libcxx/trunk/include/locale

Index: libcxx/trunk/include/locale
===
--- libcxx/trunk/include/locale
+++ libcxx/trunk/include/locale
@@ -372,19 +372,57 @@
 struct __num_get
 : protected __num_get_base
 {
-static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
 static string __stage2_float_prep(ios_base& __iob, _CharT* __atoms, _CharT& __decimal_point,
   _CharT& __thousands_sep);
-static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
-  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
-  unsigned* __g, unsigned*& __g_end, _CharT* __atoms);
+
 static int __stage2_float_loop(_CharT __ct, bool& __in_units, char& __exp,
char* __a, char*& __a_end,
_CharT __decimal_point, _CharT __thousands_sep,
const string& __grouping, unsigned* __g,
unsigned*& __g_end, unsigned& __dc, _CharT* __atoms);
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
+static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
+static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, _CharT* __atoms);
+
+#else
+static string __stage2_int_prep(ios_base& __iob, _CharT& __thousands_sep)
+{
+locale __loc = __iob.getloc();
+const numpunct<_CharT>& __np = use_facet >(__loc);
+__thousands_sep = __np.thousands_sep();
+return __np.grouping();
+}
+
+const _CharT* __do_widen(ios_base& __iob, _CharT* __atoms) const
+{
+  return __do_widen_p(__iob, __atoms);
+}
+
+
+static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, const _CharT* __atoms);
+private:
+template
+const T* __do_widen_p(ios_base& __iob, T* __atoms) const
+{
+  locale __loc = __iob.getloc();
+  use_facet(__loc).widen(__src, __src + 26, __atoms);
+  return __atoms;
+}
+
+const char* __do_widen_p(ios_base& __iob, char* __atoms) const
+{
+  (void)__iob;
+  (void)__atoms;
+  return __src;
+}
+#endif
 };
 
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
 template 
 string
 __num_get<_CharT>::__stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep)
@@ -395,6 +433,7 @@
 __thousands_sep = __np.thousands_sep();
 return __np.grouping();
 }
+#endif
 
 template 
 string
@@ -411,9 +450,16 @@
 
 template 
 int
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
 __num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
   unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
   unsigned* __g, unsigned*& __g_end, _CharT* __atoms)
+#else
+__num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, const _CharT* __atoms)
+
+#endif
 {
 if (__a_end == __a && (__ct == __atoms[24] || __ct == __atoms[25]))
 {
@@ -849,9 +895,16 @@
 // Stage 1
 int __base = this->__get_base(__iob);
 // Stage 2
-char_type __atoms[26];
 char_type __thousands_sep;
+const int __atoms_size = 26;
+#ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
+char_type __atoms1[__atoms_size];
+const char_type *__atoms = this->__do_widen(__iob, __atoms1);
+string __grouping = this->__stage2_int_prep(__iob, __thousands_sep);
+#else
+char_type __atoms[__atoms_size];
 string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep);
+#endif
 string __buf;
 __buf.resize(__buf.capacity());
 char* __a = &__buf[0];
@@ -899,9 +952,16 @@
 // Stage 1
 int __base = this->__get_base(__iob);
 // Stage 2
-char_type __atoms[26];
 char_type __thousands_sep;
+const int __atoms_size = 26;
+#ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
+char_type __atoms1[__atoms_size];
+const char_type *__atoms = this->__do_widen(__iob, __atoms1);
+string 

[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 102566.
hiraditya added a comment.

Addressed comments from Eric.


https://reviews.llvm.org/D30268

Files:
  libcxx/benchmarks/stringstream.bench.cpp
  libcxx/include/__config
  libcxx/include/locale

Index: libcxx/include/locale
===
--- libcxx/include/locale
+++ libcxx/include/locale
@@ -380,19 +380,57 @@
 struct __num_get
 : protected __num_get_base
 {
-static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
 static string __stage2_float_prep(ios_base& __iob, _CharT* __atoms, _CharT& __decimal_point,
   _CharT& __thousands_sep);
-static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
-  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
-  unsigned* __g, unsigned*& __g_end, _CharT* __atoms);
+
 static int __stage2_float_loop(_CharT __ct, bool& __in_units, char& __exp,
char* __a, char*& __a_end,
_CharT __decimal_point, _CharT __thousands_sep,
const string& __grouping, unsigned* __g,
unsigned*& __g_end, unsigned& __dc, _CharT* __atoms);
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
+static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
+static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, _CharT* __atoms);
+
+#else
+static string __stage2_int_prep(ios_base& __iob, _CharT& __thousands_sep)
+{
+locale __loc = __iob.getloc();
+const numpunct<_CharT>& __np = use_facet >(__loc);
+__thousands_sep = __np.thousands_sep();
+return __np.grouping();
+}
+
+const _CharT* __do_widen(ios_base& __iob, _CharT* __atoms) const
+{
+  return __do_widen_p(__iob, __atoms);
+}
+
+
+static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, const _CharT* __atoms);
+private:
+template
+const T* __do_widen_p(ios_base& __iob, T* __atoms) const
+{
+  locale __loc = __iob.getloc();
+  use_facet(__loc).widen(__src, __src + 26, __atoms);
+  return __atoms;
+}
+
+const char* __do_widen_p(ios_base& __iob, char* __atoms) const
+{
+  (void)__iob;
+  (void)__atoms;
+  return __src;
+}
+#endif
 };
 
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
 template 
 string
 __num_get<_CharT>::__stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep)
@@ -403,6 +441,7 @@
 __thousands_sep = __np.thousands_sep();
 return __np.grouping();
 }
+#endif
 
 template 
 string
@@ -419,9 +458,16 @@
 
 template 
 int
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
 __num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
   unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
   unsigned* __g, unsigned*& __g_end, _CharT* __atoms)
+#else
+__num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
+  unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
+  unsigned* __g, unsigned*& __g_end, const _CharT* __atoms)
+
+#endif
 {
 if (__a_end == __a && (__ct == __atoms[24] || __ct == __atoms[25]))
 {
@@ -857,9 +903,16 @@
 // Stage 1
 int __base = this->__get_base(__iob);
 // Stage 2
-char_type __atoms[26];
 char_type __thousands_sep;
+const int __atoms_size = 26;
+#ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
+char_type __atoms1[__atoms_size];
+const char_type *__atoms = this->__do_widen(__iob, __atoms1);
+string __grouping = this->__stage2_int_prep(__iob, __thousands_sep);
+#else
+char_type __atoms[__atoms_size];
 string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep);
+#endif
 string __buf;
 __buf.resize(__buf.capacity());
 char* __a = &__buf[0];
@@ -907,9 +960,16 @@
 // Stage 1
 int __base = this->__get_base(__iob);
 // Stage 2
-char_type __atoms[26];
 char_type __thousands_sep;
+const int __atoms_size = 26;
+#ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
+char_type __atoms1[__atoms_size];
+const char_type *__atoms = this->__do_widen(__iob, __atoms1);
+string __grouping = this->__stage2_int_prep(__iob, __thousands_sep);
+#else
+char_type __atoms[__atoms_size];
 string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep);
+#endif
 string 

[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Ping


https://reviews.llvm.org/D30268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

Ping


https://reviews.llvm.org/D30268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.
This revision now requires changes to proceed.

Ping


https://reviews.llvm.org/D30268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits