Page MenuHomePhabricator

Use -fomit-frame-pointer when optimizing PowerPC code
ClosedPublic

Authored by kernigh on Apr 5 2019, 12:49 PM.

Details

Summary

This enables -fomit-frame-pointer when optimizing code for all PowerPC
targets, instead of only Linux and NetBSD.

I mailed this patch to cfe-commits earlier this week.
Roman Lebedev and Hal Finkel pointed me to Phabricator;
this is my first attempt to use Phabricator.

My earlier mail wrote:

The attached patch is for clang to use -fomit-frame-pointer by default
for all PowerPC targets when optimizing code. Right now, clang uses
-fomit-frame-pointer for PowerPC Linux and NetBSD but not for other
targets. I have been running clang -target powerpc-openbsd.

The patch is for llvm-project.git master. I previously posted this
patch to https://bugs.llvm.org/show_bug.cgi?id=41094 , but the patch
in this email is for a newer revision of master.

In most functions, the frame pointer in r31 is an unnecessary extra
copy of the stack pointer in r1. GCC is using -fomit-frame-pointer by
default (in my PowerPC machine running OpenBSD/macppc); I want Clang
to be at least as good as GCC. Also, this patch helps me to compare
the output of clang -target powerpc-openbsd -O2 -S with the output
for Linux or NetBSD. In bug 41094, I showed how -fomit-frame-pointer
simplifies the C function void nothing(void) {}.

Diff Detail

Event Timeline

kernigh created this revision.Apr 5 2019, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2019, 12:49 PM

test coverage?

brad added a comment.Apr 7 2019, 3:44 PM

George, look at adding some appropriate entries to test/Driver/frame-pointer-elim.c

I'm stuck. I didn't put a test in this patch because I can't run the tests yet. So far, I can build a clang executable but can't build the rest of the project. I won't run the tests without a complete build, and I won't edit the tests without running them.

I had difficulty because llvm is a large C++ project that takes a long time to build. I tried to save time by building it on my fastest machine, an x86 desktop with 2 cores and 4G RAM running OpenBSD 6.4 amd64. I tried a parallel build in my usual way, cmake -GNinja ... then ninja, but it got stuck near the end when it tried to run 3 linkers in parallel. The linkers used too much RAM, so my machine got stuck in swap. (Do other people have more RAM?) I learned to save RAM by building the project with clang -Oz and without -g (so there is no debug info), and by using ninja -j2 clang to link only the clang executable and nothing else.

My "running OpenBSD 6.4" is a problem. Code in llvm/lib/Support/Unix/Threading.inc for OpenBSD tries to call pthread_get_name_np(), which will appear in OpenBSD 6.5. I needed to edit that file. Then I built a clang executable, but it didn't run, because of an "Out of memory" error. The kernel of OpenBSD 6.4 seems to refuse to run such a large executable. I worked around it by hacking clang into a large shared library named almost_clang, then building a tiny clang executable that just calls the shared library (by editing clang/tools/driver/{CMakeLists.txt,driver.cpp}). That caused another error, because llvm tried to set the shared library's version to "9svn", which isn't legal. I edited llvm/cmake/modules/AddLLVM.cmake to remove the shared library's version. Now I can run clang.

If I try to build the rest of the project, like ninja -j1 (because I don't want parallel linkers), then I get a linker error. It seems confused about the shared library version. The trick that I used to build clang might have broken something else. Before I try to fix my build, I want to upgrade this machine to OpenBSD 6.5 amd64. (So I am waiting for 6.5 to release; I already have 6.5-beta on my slow PowerPC machine.) That should at least let me undo the pthread_get_name_np() removal, so I don't have too many local changes. If I can build the rest of llvm and clang on OpenBSD 6.5, then I can try to run and edit the tests.

MaskRay added a subscriber: MaskRay.Jul 6 2019, 9:16 AM

I would write the following test (I can commit that for you)

diff --git c/test/Driver/frame-pointer-elim.c w/test/Driver/frame-pointer-elim.c
index 6fcd3eb75a..c2e9ccb225 100644
--- c/test/Driver/frame-pointer-elim.c
+++ w/test/Driver/frame-pointer-elim.c
@@ -1 +1,3 @@
+// DISABLE-FP-ELIM: "-mdisable-fp-elim"
+
 // For these next two tests when optimized we should omit the leaf frame
@@ -31,4 +33,3 @@
 // RUN: %clang -### -target i386-apple-darwin -S %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=DARWIN %s
-// DARWIN: "-mdisable-fp-elim"
+// RUN:   FileCheck --check-prefix=DISABLE-FP-ELIM %s
 
@@ -71,2 +72,7 @@
 
+// RUN: %clang -### -target powerpc64 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DISABLE-FP-ELIM %s
+// RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=OMIT_ALL %s
+
 void f0() {}

but I don't understand why OpenBSD enabled frame pointer elimination in rL358775 then reverted it in rL364679. It is important to know whether OpenBSD needs frame pointer elimination.

I did this in rC365862 with a test after my refactoring of the interface rC365860.

jhibbits accepted this revision.Jul 17 2019, 9:45 AM

Looks fine to me. Since it can be turned off, I don't see a problem if it causes issues.

This revision is now accepted and ready to land.Jul 17 2019, 9:45 AM
MaskRay closed this revision.Jan 10 2020, 2:40 PM