• davide

+ steven

Sure.

+ Lang, who probably has a better understanding of this, for visibility.
I don't think this should be blocked further, but he might have a chance to comment.

LGTM.

In D98458#2620731, @MaskRay wrote:

Mach-O doesn't support dso_local and this change broke XNU because of the use of dso_local.

Can you elaborate on the unsupportness? For example, the backend can ignore dso_local if it does not want to support it.

The main purpose is to reduce target differences: currently both COFF and ELF can use dso_local. If Mach-O doesn't like dso_local, there is an additional difference.

I apologize, but I don't work on this anymore.

I'm sorry, I haven't touched this code in a long time and I don't feel qualified to review it.

Committed thusly

This landed in a different form.

Somebody will pick this up, probably in a different form.

I won't have time to look at this in the future.

@labath something I noticed when finding this (and related bugs) is that frame var carries a decent diagnostic

I came to the same conclusion when analyzing https://bugs.llvm.org/show_bug.cgi?id=47257 (but you beat me to the punch).

I think this is fine. @mehdi_amini ?

In D85396#2198668, @tschuett wrote:

auto_ptr is deprecated since C++11 and will be removed in C++17.

This is correct to the best of my understanding. Thank you Jason.

yes, this makes sense. We could refine the check in future.

I assume we're still allowing to put the decorator on a test-by-test basis, and that seems the case from what I see.
If so, LG.

LGTM with Saleem's comment addressed.

https://github.com/llvm/llvm-project/commit/57605758b5de3726eec1d6e587de1003af1ab5b7

Reviewed by Jason privately.

Added the check that Jason requested.

Florian, can you take a look?

why? Do you have a testcase?

yeah, I think it's reasonable.

In D82817#2148518, @mtrofin wrote:

Tried repro-ing this on a macosx.

cmake -GNinja -DLLVM_ENABLE_MODULES=On -DCMAKE_BUILD_TYPE=Release ../llvm

ninja opt

I got a warning - missing submodule 'LLVM_Analysis.InlineSizeEstimatorAnalysis' - but no error.

Could you please point me to the specific bot, I'm probably missing something else in the cmake config - thanks!

In D82817#2148323, @davide wrote:

This broke the modules build.

In file included from <module-includes>:109:
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/Analysis/Utils/TFUtils.h:12:10: fatal error: 'tensorflow/c/c_api.h' file not found
#include "tensorflow/c/c_api.h"
         ^~~~~~~~~~~~~~~~~~~~~~
While building module 'LLVM_Backend' imported from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/lib/CodeGen/BranchFolding.h:14:
In file included from <module-includes>:3:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/AntiDepBreaker.h:20:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/ScheduleDAG.h:24:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/TargetLowering.h:34:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/SelectionDAG.h:32:
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/MachineFunction.h:27:10: fatal error: could not build module 'LLVM_Analysis'
#include "llvm/Analysis/EHPersonalities.h"
 ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

To reproduce, pass -DLLVM_ENABLE_MODULES=On. Please let me know if you need anything else to repro.

In the meanwhile, reverted this in:

commit 9908a3b9f521c954cbf6adcec35b14b2f6c8da49 (HEAD -> master, origin/master, origin/HEAD)
Author: Davide Italiano <ditaliano@apple.com>
Date:   Mon Jul 13 12:59:16 2020 -0700

    Revert "[llvm] Native size estimator for training -Oz inliner"
    
    This reverts commit 83080a294ad7d145d758821bcf4354ad0cb7d299 as
    it breaks the macOS modules build.

This broke the modules build.

Good, one minor suggestion.

LGTM

Reverted in:

on any recent'ish macOS (although, I don't think the OS quite matters)

This broke the modules build on macOS.

This broke macOS: