[llvm-dev] Removing LLVM_ALWAYS_INLINE from ADT classes

[Nico Weber via llvm-dev]

On Mon, Jan 14, 2019 at 6:17 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

> The *specific* desire is to get methods from Support and ADT to work well
> in debug builds. So not sure building those things with O3 helps.
>
> I really don't understand the tension here.
>
> Fundamentally, I think we need to give up on debug builds have super
> predictable build-time performance. IMO, that includes tablegen. I'm happy
> to have workarounds like building an optimized tablegen binary, but I'm not
> sure its worth the complexity that it sounsd like that entails.
>
> (Separately, I really wish Ninja and/or CMake had *good* models for a host
> build vs. a target build so that it was easy and reliable to get optimized
> host utilities built as well as unoptimized target binaries, but that seems
> to be a real problem in their current design and I'm not suggesting we wait
> for that to fix any of this.)
>

(That's a CMake issue. See https://reviews.llvm.org/D56577
llvm/trunk/utils/gn/secondary/BUILD.gn line 15 for how you say "build this
target, but with a toolchain targeting Android instead of the host system"
in GN, and GN just writes ninja files too – so this is very possible on a
Ninja level.)


>
> On Mon, Jan 14, 2019 at 3:11 PM Nico Weber <thakis at chromium.org> wrote:
>
>> I agree that shelling out to `cmake --build` as a build step is pretty
>> ugly.
>>
>> It seems kind of simpler to me to just always build Support and TableGen
>> with O3 always, even in debug builds. Most people don't debug code in
>> Support and TableGen, and there could be a disable switch for those that
>> do. That gives you a fast(er) debug build without building any files twice.
>> Is that something CMake can do? If so, is this something that was
>> considered?
>>
>> (It's not implemented, but in the GN build it's in theory doable to have
>> a release toolchain and always use that for tablegen, that way you have the
>> same effect, but the duplicate build of tablegen is just part of the
>> regular build. But I figured the "always O3 for lib/Support and
>> lib/TableGen" approach would be nicer there too. Since I always do Release
>> builds like Reid and just switch to Debug when I need it, it wasn't a
>> priority for me to think too much about this though.)
>>
>> On Mon, Jan 14, 2019 at 4:52 PM Reid Kleckner <rnk at google.com> wrote:
>>
>>> On Fri, Jan 11, 2019 at 11:18 AM Davide Italiano via llvm-dev <
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>>> After yet another round of discussions, the plan is that of trying not
>>>> to slap another attribute on the members, instead going for making
>>>> OPTIMIZED_TLBGEN the default and removing always_inline.
>>>> I'll do some testing locally (for the Ninja and the Xcode build
>>>> generator). Pawel volunteered to take a look at the Visual Studio
>>>> situation.
>>>> I assume this is going to be the final plan we'll deploy unless
>>>> somebody comes with any fundamental objections or there are technical
>>>> hurdles down the road.
>>>>
>>>
>>> So, personally I feel strongly that LLVM_OPTIMIZED_TABLEGEN should be
>>> off by default, since the it splits up the ninja build graph. We've also
>>> run into issues with it with VS: https://reviews.llvm.org/D54153
>>> Shelling out to `cmake --build` as part of a build action doesn't seem to
>>> work very well for many generators.
>>>
>>> What actually blocks us from removing these always_inline attributes?
>>> When I read this thread, I didn't get the sense that debug build tablegen
>>> performance was actually important to anyone, but maybe I skimmed too much.
>>>
>>> Maybe I am a luddite because I just build Release+Asserts with debug
>>> info enabled and then sprinkle `#pragma clang optimize off` around when I
>>> need to debug a particular source file. This keeps build & test fast enough
>>> except in the corner cases where a debugger is required.
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190114/870e0ff7/attachment.html>