[PATCH] D30791: Add support for -fno-builtin to LTO and ThinLTO on Darwin

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D30791#697755, @tejohnson wrote:

> 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.



================
Comment at: llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h:146
                        bool DisableInline, bool DisableGVNLoadPRE,
-                       bool DisableVectorization);
+                       bool DisableVectorization, bool DisableLTOBuiltins);
 
----------------
tejohnson wrote:
> Why not instead add this flag as a member of LTOCodeGenerator, similar to what you do for the ThinLTOCodeGenerator?
I just tried to fit into the existing by mimicking `DisableVectorization`. That said I think I can do differently (see below).


================
Comment at: llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h:209
 
+  void setNoBuiltin(bool NoBuiltin) { DisableLTOBuiltins = NoBuiltin; }
+
----------------
tejohnson wrote:
> document
Will do!


================
Comment at: llvm/test/ThinLTO/X86/tli-nobuiltin.ll:38
+
+; Check fprintf(fp, "%s", str) -> fputs(str, fp) only when builtins are enabled
+
----------------
tejohnson wrote:
> s/fputs/fwrite/ per earlier checks?
Oh right!


================
Comment at: llvm/tools/lto/lto.cpp:165
     report_fatal_error("Optimization level must be between 0 and 3");
   CG->setOptLevel(OptLevel - '0');
 }
----------------
I likely can set the `DisableLTOBuiltins ` on the CodeGenerator here.


https://reviews.llvm.org/D30791