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

2023-05-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D150490#4343442 , @craig.topper 
wrote:

> In D150490#4343145 , @enh wrote:
>
>> In D150490#4343128 , @hiraditya 
>> wrote:
>>
 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.
>>
>> well, when i said that on the internal chat, i thought you were asking "why 
>> do we say what clang already says?" :-)
>>
>> if the question was actually "is there more context on why Android enables 
>> the frame pointer?" i'd have said something like "because Android developers 
>> [OS and app developers alike] do so much debugging from the field, where all 
>> we get is a crash report for something we probably can't repro locally, that 
>> having good _and_ cheap unwinds is super important to us".
>
> Thanks. I suspected that was the answer, but I wanted to check that it made 
> sense to copy AArch64.

Enabling the frame pointer is fine. We probably should revert ancient 
conventions by using `-fasynchronous-unwind-tables`...


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

2023-05-15 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D150490#4343145 , @enh wrote:

> In D150490#4343128 , @hiraditya 
> wrote:
>
>>> 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.
>
> well, when i said that on the internal chat, i thought you were asking "why 
> do we say what clang already says?" :-)
>
> if the question was actually "is there more context on why Android enables 
> the frame pointer?" i'd have said something like "because Android developers 
> [OS and app developers alike] do so much debugging from the field, where all 
> we get is a crash report for something we probably can't repro locally, that 
> having good _and_ cheap unwinds is super important to us".

Thanks. I suspected that was the answer, but I wanted to check that it made 
sense to copy AArch64.


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

2023-05-15 Thread Elliott Hughes via Phabricator via cfe-commits
enh added a comment.

In D150490#4343128 , @hiraditya wrote:

>> 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.

well, when i said that on the internal chat, i thought you were asking "why do 
we say what clang already says?" :-)

if the question was actually "is there more context on why Android enables the 
frame pointer?" i'd have said something like "because Android developers [OS 
and app developers alike] do so much debugging from the field, where all we get 
is a crash report for something we probably can't repro locally, that having 
good _and_ cheap unwinds is super important to us".


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

2023-05-15 Thread 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

2023-05-15 Thread 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

2023-05-15 Thread 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 rGf98698714e2e: Enable frame pointer for all non-leaf 
functions on riscv64 Android (authored by AdityaK 
1894981+hiradi...@users.noreply.github.com).

Changed prior to commit:
  https://reviews.llvm.org/D150490?vs=521835=522255#toc

Repository:
  rG LLVM Github Monorepo

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
+++ 

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

2023-05-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



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) ||

craig.topper wrote:
> You can use Triple.isRISCV64()
&& has a lower precedence than `==`. `==` doesn't need parens.



Comment at: clang/test/Driver/frame-pointer-elim.c:109
 // 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

Use `--target=` for new tests. `-target ` has been deprecated since clang 3.x, 
albeit without a warning.



Comment at: clang/test/Driver/frame-pointer.c:61
+// 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

I don't think we need so many RUN lines. `-O0` and `-O1` suffice.


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

2023-05-12 Thread Elliott Hughes via Phabricator via cfe-commits
enh accepted this revision.
enh added a comment.
This revision is now accepted and ready to land.

(lgtm with craig.topper's suggested simplification.)




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) ||

hiraditya wrote:
> enh wrote:
> > how does this work for Android/arm64?
> becaues of `Triple.isAArch64()`, AArch64 always has non-leaf frame pointers 
> for all platforms.
lol. i could seem so much smarter if only i'd learn to read :-)


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

2023-05-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



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) ||

You can use Triple.isRISCV64()


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

2023-05-12 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

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


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

2023-05-12 Thread 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

2023-05-12 Thread 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

2023-05-12 Thread 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

2023-05-12 Thread 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

2023-05-12 Thread 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

2023-05-12 Thread Elliott Hughes via Phabricator via cfe-commits
enh 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()) {

(where? is it simpler to just add arm64 to the switch?)



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) ||

how does this work for Android/arm64?


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

2023-05-12 Thread 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

2023-05-12 Thread Elliott Hughes via Phabricator via cfe-commits
enh added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:423
 
+  if (Triple.isAndroid() && Triple.getArch() == llvm::Triple::riscv64)
+return true;

should this...



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:439
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
   case llvm::Triple::sparc:

(ah, this is why you need Android early. but, yeah, probably worth moving all 
the Android stuff together, like with the other OSes?)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:459
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.

...be down here instead?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:465
 case llvm::Triple::thumbeb:
   if (Triple.isAndroid())
 return true;

(take care of all this android/arm stuff where you take care of 
android/riscv64? arm64 has frame pointers by "default default", so we don't 
need to mention it?)


Repository:
  rG LLVM Github Monorepo

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

2023-05-12 Thread 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