[PowerPC] Make no-PIC default to match GCC - CLANG

[stefanp]

Summary

Ran a quick test and determined that for GCC the default option is -fno-PIC for both ELFv1 and ELFv2. We are currently using -fPIC as the default for LLVM.
Would like to revert to what GCC is doing on PowerPC.

NOTE: This patch does not yet include test case updates. Currently working on those updates and will further update this patch at that time. In the meantime I put up this patch to make sure that this is in fact what we want to do.

[nemanjai]

TL; DR: +1 from me for proceeding with this patch. If you don't hear any strong objections before next week, I'd say please update the test cases and we'll go ahead with this.

I think it is important to match what GCC is doing for two reasons.

1. This is just what users have come to expect and will typically use -fPIC/-fpic explicitly when they rely on this behaviour.
2. Most build systems that build both static and shared versions will use -fPIC explicitly for the shared library case and just omit the option (rather than adding -fno-pic for the static library case).

For the most part, things work the same on PPC between the two cases, but there are some important differences - not the least of which being tail-call optimizations. We do not want the PIC default to take away those optimization opportunities in builds where PIC truly doesn't matter.

[stefanp]

Test cases updated.

[stefanp]

Default on all Power PC platforms is now no-PIC.
If any bugs are exposed by this change we will fix them as they are exposed.

[jsji]

Agree, I prefer that we don't set either -fPIC or -fPIE by default .

test/Driver/ppc-abi.c

33 ↗

(On Diff #170926)

No sure whether this is correct place to add, but maybe we should add some test to make sure that adding -fPIC option will set the pic-level the same as before?

test/Driver/print-multi-directory.c

7 ↗

(On Diff #170926)

Do we know why this changes due to fPIC default change?
If this is a non-relevant change, maybe deliver in another NFC commit? Thanks.

[stefanp]

test/Driver/ppc-abi.c

33 ↗

(On Diff #170926)

Good point. I'm going to add another option to check that -fPIC behaves as it did before.

test/Driver/print-multi-directory.c

7 ↗

(On Diff #170926)

No I don't know exactly why this changes.
I was actually debating in this situation whether or not I should just add -fPIC to the options to keep the behaviour the same as before.

Unfortunately I can't put this in a later NFC change because I would leave a failing test. I either have to adjust the test or I would have to add -fPIC.

[sfertile]

test/Driver/print-multi-directory.c

7 ↗

(On Diff #170926)

This test changing behavior is suspect, the run line uses a target that is not powerpc and if the test somehow depended on the hosts default pic value wouldn't it be failing for mips64 as well? FWIW I applied this change to top-of-trunk and ran the test an X86 machine and the test behavior didn't change. I think it worth investigating to find out why this test changes behavior due to this change.

[nemanjai]

test/Driver/print-multi-directory.c

7 ↗

(On Diff #170926)

@stefanp You need to figure out what is going on with this test case for you. I have tried this on PPC as well and the test case did *not* change behaviour.

[stefanp]

test/Driver/print-multi-directory.c

7 ↗

(On Diff #170926)

I got to the bottom of this issue.
It was a mistake on my part. The snapshot that I had for llvm had that test failing before I added my change.
This test has nothing to do with my change at all and I so won't modify it.
Sorry for the confusion... :(

[stefanp]

Added the PIC version to the second test as well.
Removed the third test. It has nothing to do with this change and I should not have modified that test in the first place.

[sfertile]

LGTM

[stefanp]

Patch is now only for Little Endian.
Big Endian will remain PIC by default as it was before. This is due to a bug found on BE where -fsanitize=leak is not detecting leaks correctly. As Little Endian is a higher priority we will add that code first and then look at the BE issue at a later date.

[sfertile]

cfe/trunk/lib/Driver/ToolChains/Gnu.cpp

2432–2433

I think you should mention why BE has to remain pic by default in this comment.

cfe/trunk/lib/Driver/ToolChains/Gnu.cpp

 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList &Args) const {
   return getArch() == llvm::Triple::x86_64;
 }
 
 bool Generic_GCC::isPICDefault() const {
   switch (getArch()) {
   case llvm::Triple::x86_64:
     return getTriple().isOSWindows();
-  case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
-    return !getTriple().isOSBinFormatMachO() && !getTriple().isMacOSX();
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el:
     return true;
   default:
     return false;
   }
 }
 
 bool Generic_GCC::isPIEDefault() const { return false; }

cfe/trunk/test/Driver/clang-offload-bundler.c

 // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLE____START__ openmp-powerpc64le-ibm-linux-gnu
 // CK-TEXTI: Content of device file 1
 // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLE____END__ openmp-powerpc64le-ibm-linux-gnu
 // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLE____START__ openmp-x86_64-pc-linux-gnu
 // CK-TEXTI: Content of device file 2
 // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLE____END__ openmp-x86_64-pc-linux-gnu
 
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLE____START__ host-powerpc64le-ibm-linux-gnu
-// CK-TEXTLL: @A = global i32 0
+// CK-TEXTLL: @A = dso_local global i32 0
 // CK-TEXTLL: define {{.*}}@test_func()
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLE____END__ host-powerpc64le-ibm-linux-gnu
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLE____START__ openmp-powerpc64le-ibm-linux-gnu
 // CK-TEXTLL: Content of device file 1
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLE____END__ openmp-powerpc64le-ibm-linux-gnu
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLE____START__ openmp-x86_64-pc-linux-gnu
 // CK-TEXTLL: Content of device file 2
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLE____END__ openmp-x86_64-pc-linux-gnu

cfe/trunk/test/Driver/ppc-abi.c

 // RUN:   | FileCheck -check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   -mabi=altivec | FileCheck -check-prefix=CHECK-ELFv2 %s
 
-// CHECK-ELFv1: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv1: "-mrelocation-model" "static"
 // CHECK-ELFv1: "-target-abi" "elfv1"
-// CHECK-ELFv1-QPX: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv1-QPX: "-mrelocation-model" "static"
 // CHECK-ELFv1-QPX: "-target-abi" "elfv1-qpx"
-// CHECK-ELFv2: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv2: "-mrelocation-model" "static"
 // CHECK-ELFv2: "-target-abi" "elfv2"
 
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv1-qpx | FileCheck -check-prefix=CHECK-ELFv1-QPX-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mcpu=a2q | FileCheck -check-prefix=CHECK-ELFv1-QPX-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mcpu=a2 -mqpx | FileCheck -check-prefix=CHECK-ELFv1-QPX-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mcpu=a2q -mno-qpx | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2-PIC %s
+
+// RUN: %clang -fPIC -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv2-PIC %s
+// RUN: %clang -fPIC -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2-PIC %s
+// RUN: %clang -fPIC -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=altivec | FileCheck -check-prefix=CHECK-ELFv2-PIC %s
+
+// CHECK-ELFv1-PIC: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv1-PIC: "-target-abi" "elfv1"
+// CHECK-ELFv1-QPX-PIC: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv1-QPX-PIC: "-target-abi" "elfv1-qpx"
+// CHECK-ELFv2-PIC: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv2-PIC: "-target-abi" "elfv2"
+
+