Page MenuHomePhabricator

Add support for -fno-builtin to LTO and ThinLTO on Darwin
ClosedPublic

Authored by mehdi_amini on Mar 9 2017, 1:47 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mehdi_amini created this revision.Mar 9 2017, 1:47 PM
tejohnson edited edge metadata.Mar 10 2017, 7:45 AM

It looks like this is part of the required fix for PR30403? I guess the other part is to add the module flag to be set during the FE, and use that to set this flag?

It would be good to add support in the new LTO API too.

In the discussion on PR30403 it sounds like the desired behavior was closer to -ffreestanding than -fno-builtin, so should the variable names reflect that?

llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
146 ↗(On Diff #91215)

Why not instead add this flag as a member of LTOCodeGenerator, similar to what you do for the ThinLTOCodeGenerator?

llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
209 ↗(On Diff #91215)

document

llvm/test/ThinLTO/X86/tli-nobuiltin.ll
38 ↗(On Diff #91215)

s/fputs/fwrite/ per earlier checks?

It looks like this is part of the required fix for PR30403? I guess the other part is to add the module flag to be set during the FE, and use that to set this flag?

This is a lame but pragmatic way of going around the issue and allow our kernel to be built with ThinLTO ;)
The plan for the "clean solution" in the future is to embed the information in function-level attributes. But it'll take too much time for me to accomplish right now (and in the 5.0 timeframe FWIW).

It would be good to add support in the new LTO API too.

In the discussion on PR30403 it sounds like the desired behavior was closer to -ffreestanding than -fno-builtin, so should the variable names reflect that?

The discussion on clang-dev blurred the lines between -ffreestanding and -fno-builtin, to the point where I gave up trying to differentiate them.

llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
146 ↗(On Diff #91215)

I just tried to fit into the existing by mimicking DisableVectorization. That said I think I can do differently (see below).

llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
209 ↗(On Diff #91215)

Will do!

llvm/test/ThinLTO/X86/tli-nobuiltin.ll
38 ↗(On Diff #91215)

Oh right!

llvm/tools/lto/lto.cpp
165 ↗(On Diff #91215)

I likely can set the DisableLTOBuiltins on the CodeGenerator here.

pcc edited edge metadata.Mar 13 2017, 2:56 PM

If this is just a temporary solution for one specific application, does it really need first class support in the clang driver? Would it not be sufficient for you to pass -Wl,-mllvm,-lto-nobuiltin at link time?

I also have a mild preference to use the term "freestanding" for this.

In D30791#699844, @pcc wrote:

If this is just a temporary solution for one specific application, does it really need first class support in the clang driver? Would it not be sufficient for you to pass -Wl,-mllvm,-lto-nobuiltin at link time?

Sure. I'm fine dropping the clang part.

I also have a mild preference to use the term "freestanding" for this.

I don't mind.

mehdi_amini marked 5 inline comments as done.

Address comments:

  • set Freestanding as a member of LTOCodegenerator
  • document the setter
  • drop the clang driver changes
mehdi_amini marked 2 inline comments as done.Mar 28 2017, 10:02 AM

clang-format

pcc accepted this revision.Mar 28 2017, 10:28 AM

You may want to include the freestanding flag in the code generation options hash.

Aside from that, LGTM.

This revision is now accepted and ready to land.Mar 28 2017, 10:28 AM
This revision was automatically updated to reflect the committed changes.
In D30791#712296, @pcc wrote:

You may want to include the freestanding flag in the code generation options hash.

done before committing