clang/AMDGPU: Fix default for frame-pointer attribute

[arsenm]

Summary

Enabling optimization should allow frame pointer elimination.

[arsenm]

Ping

[yaxunl]

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?

[t-tye]

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

[scott.linder]

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.

[arsenm]

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

[arsenm]

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

[arsenm]

2cc11941a2e88236e0b4842229454ae6d85142cd

clang/lib/Driver/ToolChains/Clang.cpp

case llvm::Triple::msp430:
     // XCore never wants frame pointers, regardless of OS.
     // WebAssembly never wants frame pointers.
     return false;
   case llvm::Triple::ppc:
   case llvm::Triple::ppc64:
   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;
   }
 
   if (Triple.isOSNetBSD()) {
     return !areOptimizationsEnabled(Args);
   }

clang/test/Driver/frame-pointer-elim.cl

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