[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-12-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

2cc11941a2e88236e0b4842229454ae6d85142cd 



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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-27 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D70424#1758902 , @yaxunl wrote:

> LGTM. But I am wondering how it affects -g. Do we need to keep frame pointer 
> when -g is specified? Should we add a test for -O3 -g?


It’s independent for every other target. This matches the standard behavior


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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-27 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D70424#1761298 , @arsenm wrote:

> In D70424#1758902 , @yaxunl wrote:
>
> > LGTM. But I am wondering how it affects -g. Do we need to keep frame 
> > pointer when -g is specified? Should we add a test for -O3 -g?
>
>
> It’s independent for every other target. This matches the standard behavior


Also -g should not change codegen, but FP elimination would obviously be a 
change


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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-26 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment.

In D70424#1759379 , @t-tye wrote:

> @scott.linder can answer about the -g question, but I would expect that the 
> CFI is capable of describing the address of the CFA regardless of whether 
> there is a frame pointer by simply knowing the constant offset from the stack 
> pointer.
>
> For AMDGPU it seems to me what we really have is an FP and we optimize away 
> the SP since the stack grows low address to high address, and S32 points to 
> the base of the frame, and not the top of the stack.


Right, with CFI we will be able to describe every case we can generate code 
for, regardless of whether we optimize out the FP.


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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-25 Thread Tony Tye via Phabricator via cfe-commits
t-tye added a comment.

@scott.linder can answer about the -g question, but I would expect that the CFI 
is capable of describing the address of the CFA regardless of whether there is 
a frame pointer by simply knowing the constant offset from the stack pointer.

For AMDGPU it seems to me what we really have is an FP and we optimize away the 
SP since the stack grows low address to high address, and S32 points to the 
base of the frame, and not the top of the stack.


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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

LGTM. But I am wondering how it affects -g. Do we need to keep frame pointer 
when -g is specified? Should we add a test for -O3 -g?


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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-24 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

Ping


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

https://reviews.llvm.org/D70424



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


[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

2019-11-19 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm created this revision.
arsenm added reviewers: yaxunl, scott.linder, t-tye.
Herald added subscribers: tpr, dstuttard, wdng, kzhuravl.

Enabling optimization should allow frame pointer elimination.


https://reviews.llvm.org/D70424

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


Index: clang/test/Driver/frame-pointer-elim.cl
===
--- /dev/null
+++ clang/test/Driver/frame-pointer-elim.cl
@@ -0,0 +1,8 @@
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 %s -o %t.s 2>&1 | 
FileCheck -check-prefix=CHECKNONE %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 -fno-omit-frame-pointer 
%s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S %s -o %t.s 2>&1 | FileCheck 
-check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O0 %s -o %t.s 2>&1 | 
FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -cl-opt-disable %s -o %t.s 
2>&1 | FileCheck -check-prefix=CHECKALL %s
+
+// CHECKNONE: -mframe-pointer=none
+// CHECKALL: -mframe-pointer=all
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -538,6 +538,8 @@
   case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+  case llvm::Triple::amdgcn:
+  case llvm::Triple::r600:
 return !areOptimizationsEnabled(Args);
   default:
 break;


Index: clang/test/Driver/frame-pointer-elim.cl
===
--- /dev/null
+++ clang/test/Driver/frame-pointer-elim.cl
@@ -0,0 +1,8 @@
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKNONE %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O3 -fno-omit-frame-pointer %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -O0 %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+// RUN: %clang -target amdgcn-amd-amdhsa -### -S -cl-opt-disable %s -o %t.s 2>&1 | FileCheck -check-prefix=CHECKALL %s
+
+// CHECKNONE: -mframe-pointer=none
+// CHECKALL: -mframe-pointer=all
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -538,6 +538,8 @@
   case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+  case llvm::Triple::amdgcn:
+  case llvm::Triple::r600:
 return !areOptimizationsEnabled(Args);
   default:
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits