Linaro / OpenCSD

CoreSight trace stream decoder developed openly
https://github.com/Linaro/opencsd/wiki
Other
141 stars 53 forks source link

Use -fPIC rather than -fpic #16

Closed onitake closed 5 years ago

onitake commented 5 years ago

Due to fine implementation differences between -fpic and -fPIC on certain CPU architectures, the decoder library currently fails to build on Debian sparc64: https://buildd.debian.org/status/fetch.php?pkg=libopencsd&arch=sparc64&ver=0.10.1-1&stamp=1552450383&raw=0

Since Debian builds packages on all supported architectures by default and other packages depend on libopencsd, this has certain unpleasant consequences. A possible fix would be to remove those dependencies for sparc64, but it would be much cleaner to fix the build script instead.

Is it possible to change the compiler flags from -fpic to -fPIC here or is there a specific reason why -fpic is used?

glaubitz commented 5 years ago

I have just tested this on sparc64 and can confirm that replacing -fpic with -fPIC fixes the issue for me.

mikel-armbb commented 5 years ago

Hi,

(+coresight mailing lists.)

Looked at this - -fpic is supposed to generate smaller code then -fPIC. That said, I've tried both variants for x86_64 and aarch64 builds:

x86_64 showed no change, (gcc 5.4) cross compiled aarch64 code was 0.45% smaller using -fpic rather than -fPIC. (gcc 6.2) native compiled aarch64 code showed no change (gcc 4.9)

While we could add some code to the makefile to dynamically change the -fPIC/pic option when building on sparc architectures, unless there are objections on the mailing list, I propose to change to -fPIC across the board at this point.

This will be released as a 0.11.1 patch (along with another minor build fix.)

Regards

Mike

On Wed, 13 Mar 2019 at 08:50, John Paul Adrian Glaubitz < notifications@github.com> wrote:

I have just tested this on sparc64 and can confirm that replacing -fpic with -fPIC fixes the issue for me.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Linaro/OpenCSD/issues/16#issuecomment-472332457, or mute the thread https://github.com/notifications/unsubscribe-auth/AMvwsxbzERGcBbzJECyGHDUxntbh6moBks5vWLvkgaJpZM4bsquw .

-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

wookey commented 5 years ago

Cheers for reporting this upstream Adrian. As we are in freeze and this isn't RC I won't upload a fix to debian yet (although I've just done one). I'll wait for 0.11 and upload that in due course.

mikel-armbb commented 5 years ago

Requested change implemented on release v0.11.1

Thanks

Mike

onitake commented 5 years ago

Thanks a lot, @mikel-armbb !