This is an archive of the discontinued LLVM Phabricator instance.

Disable PIE
Needs ReviewPublic

Authored by foad on Jan 3 2023, 4:11 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

This speeds up lit testing on systems where the host toolchain enables
PIE by default. For example on my Ubuntu 22.04 system, using the host's
default clang and lld, this speeds up check-llvm by around 1.75x.

Diff Detail

Event Timeline

foad created this revision.Jan 3 2023, 4:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 4:11 AM
foad requested review of this revision.Jan 3 2023, 4:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 4:11 AM
arsenm added a subscriber: arsenm.Jan 20 2023, 8:35 AM

Can you elaborate on the scope of what builds this change touches in the commit description?

I think we should do this, but there should probably be a build option to use the default behavior or an explicit use PIE

You probably want https://cmake.org/cmake/help/v3.16/module/CheckPIESupported.html
That being said, i don't think this can be just set, at best it should be hidden under an opt-in flag.

foad added a comment.Jan 20 2023, 9:08 AM

That being said, i don't think this can be just set, at best it should be hidden under an opt-in flag.

I don't think there's much point doing this at all if it's opt-in. If you already know that you're suffering from a slow development cycle due to PIE, you might as well just type cmake -DCMAKE_EXE_LINKER_FLAGS=-no-pie. The idea of doing by default was to fix the problem for developers who didn't even know they had the problem.

That being said, i don't think this can be just set, at best it should be hidden under an opt-in flag.

I don't think there's much point doing this at all if it's opt-in. If you already know that you're suffering from a slow development cycle due to PIE, you might as well just type cmake -DCMAKE_EXE_LINKER_FLAGS=-no-pie. The idea of doing by default was to fix the problem for developers who didn't even know they had the problem.

There is a point because it requires I know the flag for this, the cmake flag for this, and need to know I should think about this in the first place. This should be an option prominently advertised in the llvm cmake usage documentation and put in the developer guide

That being said, i don't think this can be just set, at best it should be hidden under an opt-in flag.

I don't think there's much point doing this at all if it's opt-in. If you already know that you're suffering from a slow development cycle due to PIE, you might as well just type cmake -DCMAKE_EXE_LINKER_FLAGS=-no-pie. The idea of doing by default was to fix the problem for developers who didn't even know they had the problem.

Right. I get that. The problem is that this *is* going to affect everyone,
including distributions, that wanted PIE to be the default in the first place.
So once again, i don't think that's generally viable.

foad added a comment.Jan 20 2023, 9:15 AM

There is a point because it requires I know the flag for this, the cmake flag for this, and need to know I should think about this in the first place. This should be an option prominently advertised in the llvm cmake usage documentation and put in the developer guide

My point was that an off-by-default flag would not help you "to know I should think about this in the first place". But it hadn't occurred to me that we could document it, and people would actually read that. :-)

The problem is that this *is* going to affect everyone, including distributions, that wanted PIE to be the default in the first place.

I accept that there is probably conflict between what developers want and what distributors want. This patch was my way of voting for the developers. :-)

There is a point because it requires I know the flag for this, the cmake flag for this, and need to know I should think about this in the first place. This should be an option prominently advertised in the llvm cmake usage documentation and put in the developer guide

My point was that an off-by-default flag would not help you "to know I should think about this in the first place". But it hadn't occurred to me that we could document it, and people would actually read that. :-)

We probably should have an example "this is the set of flags you actually want" there too. I spent way too much time when I'm on a new system figuring out the full set of LLVM_USE_LINKER=lld and the link threads options and all the others you need for fast builds