In D94451#2503043, @tstellar wrote:

In D94451#2502977, @GMNGeoffrey wrote:

[snip]

I feel like we've been side-tracked by meta-arguments a lot in this discussion, so I'm going to make a few points and hopefully try to wrap this up.

In D94451#2502917, @tstellar wrote:

In D94451#2502765, @GMNGeoffrey wrote:

In D94451#2496546, @tstellar wrote:

In D94451#2496129, @GMNGeoffrey wrote:

In D94451#2491553, @ctetreau wrote:

Question: Would there be an expectation that unsupported build systems are in a good state for LLVM version branches? If I checkout the git tag for a final LLVM release, should I be able to expect that the build system is functional at that commit?

No. I think there should be no expectation. If the community members who care about the component can figure out a way to make this happen that doesn't make releasing harder for the release manager then they could try to arrange this, but I suspect that's not really possible. I think if that needs to be clearer, it should be clarified in the support policy. https://llvm.org/docs/SupportPolicy.html#peripheral-tier mentions things that aren't "regularly released", but I think that zero expectation of support in releases could be made clearer. Assuming that's already the status quo, I could send it as a patch to the support policy separate from this proposal. Or I could include in this proposal that we'll amend the support policy to reflect that.

In D94451#2495535, @rengolin wrote:

In D94451#2491668, @tstellar wrote:

If this pitch is accepted, then what are the next steps required in order for bazel or another build system to be added to tree? For example, can they automatically be added if they meet the support policy requirements or does there need to be some kind of community ack for? I think the pitch needs to be more clear about what this process is.

This is addressed by https://llvm.org/docs/SupportPolicy.html#inclusion-policy

Yes I'm trying not to duplicate information already in the support policy, where possible.

OK, I think it would be better if this pitch was about adding bazel specifically. I don't really see what having a generic "unsupported build system" pitch gains us, when the specific build systems will all still need to go through the RFC process.

So I must confess that I'm a bit frustrated by this comment, Tom. The reason I'm writing this as a pitch is because the RFC(s) I wrote were determined to be "controversial" even after we had a support policy that explicitly discussed build systems. As far as I can tell, none of the objections raised that made it controversial had to do with Bazel in particular or whether its addition could satisfy the requirements of the support policy, but rather had to do with adding unsupported build systems in general. That was surprising to me, since we'd just had a separate RFC for a support policy, which seems like it would have been the ideal time to raise such objections, but people made the reasonable argument that the writing of that policy was very new (although the policies it recorded were not) and the inclusion of build configurations might reasonably have been missed when people were reviewing it. The prevailing view on the thread seemed to be we should escalate the point about unsupported build systems to come to a decision. So in my mind, this pitch is to make explicit and allow further discussion on the inclusion of unsupported build systems in the support policy, whereas before it was just a sentence that mentioned them as an example. I'm sympathetic to that and don't want to be just slipping things through, but I'd also like to actually make progress here and I feel like we have entered exactly the situation you've described as being opposed to when requesting that we escalate to a pitch: that is a "battle of attrition". I think you've raised reasonable concerns about build system proliferation, and I think we should discuss them, but I'm not sure how me writing yet another iteration of this proposal actually helps us here.

I guess I'm fine making this proposal "Unsupported build systems are allowed and Bazel is, in particular", so we don't have to then do yet another RFC, but my plan was to just reopen the previous RFC and return to the issues of Bazel in particular (and even more specifically our implementation of the LLVM config for it, which I think there were some reasonable criticisms of). I'm worried that discussing the specific case means that if some poor soul wants to add, say, a Meson build or whatever they will have to relitigate these points that have already been decided.

Can you understand where I'm coming from here?

Yes, and I think there may be some misunderstanding here, because the main reason I suggested you include Bazel in this pitch was to try to save your time and energy.

When I first suggested doing a pitch, my assumption was that the RFC for Bazel would turn into a pitch for Bazel and that would be it, however if we proceed with this pitch as written, then we would have had 2 RFCs, 1 Pitch, and still no final decision about Bazel. This seems to me like much more work then what we need to do, not to mention the fact that it is very confusing, and I don't think it's really fair to expect people who object to the original RFC to keep objecting to all these new proposals.

In D94451#2496546, @tstellar wrote:

In D94451#2496129, @GMNGeoffrey wrote:

In D94451#2491553, @ctetreau wrote:

Question: Would there be an expectation that unsupported build systems are in a good state for LLVM version branches? If I checkout the git tag for a final LLVM release, should I be able to expect that the build system is functional at that commit?

No. I think there should be no expectation. If the community members who care about the component can figure out a way to make this happen that doesn't make releasing harder for the release manager then they could try to arrange this, but I suspect that's not really possible. I think if that needs to be clearer, it should be clarified in the support policy. https://llvm.org/docs/SupportPolicy.html#peripheral-tier mentions things that aren't "regularly released", but I think that zero expectation of support in releases could be made clearer. Assuming that's already the status quo, I could send it as a patch to the support policy separate from this proposal. Or I could include in this proposal that we'll amend the support policy to reflect that.

In D94451#2495535, @rengolin wrote:

In D94451#2491668, @tstellar wrote:

If this pitch is accepted, then what are the next steps required in order for bazel or another build system to be added to tree? For example, can they automatically be added if they meet the support policy requirements or does there need to be some kind of community ack for? I think the pitch needs to be more clear about what this process is.

This is addressed by https://llvm.org/docs/SupportPolicy.html#inclusion-policy

Yes I'm trying not to duplicate information already in the support policy, where possible.

OK, I think it would be better if this pitch was about adding bazel specifically. I don't really see what having a generic "unsupported build system" pitch gains us, when the specific build systems will all still need to go through the RFC process.

• Fix typos

In D94451#2491553, @ctetreau wrote:

Question: Would there be an expectation that unsupported build systems are in a good state for LLVM version branches? If I checkout the git tag for a final LLVM release, should I be able to expect that the build system is functional at that commit?

• Add links to pitch thread now that it exists

FYI this leaves an unused function

In D91762#2406552, @GMNGeoffrey wrote:

FYI this results in unused private fields which causes diagnostic errors for -Wunused-private-field.

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:365:13: error: private field 'GetArgTLS' is not used [-Werror,-Wunused-private-field]
  Constant *GetArgTLS;
            ^
llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:366:13: error: private field 'GetRetvalTLS' is not used [-Werror,-Wunused-private-field]
  Constant *GetRetvalTLS;

FYI this results in unused private fields which causes diagnostic errors for -Wunused-private-field.

• Add coverage info and usage instructions

• Add README
• Pull in new changes from llvm-bazel repository

Thanks for adding this Renato :-) I was out the first part of this week, but LGTM

LGTM, but please wait for others' review, obviously :-)

Overall LGTM, with some wording nits. One thing I think is missing is guidance for how a peripheral component gets added. Experimental targets already have their own documentation, and my impression is that editor bindings are just a matter of sending a patch (which seems reasonable to me). What about something like:

https://github.com/llvm/llvm-project/commit/1142eaed9d4e31aa10584f2755a350970495002f reverts

In D90477#2365523, @antiagainst wrote:

In D90477#2365513, @GMNGeoffrey wrote:

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

Oops. Thanks Geoffrey! If it's not trivial, feel free to revert.

In D90477#2365513, @GMNGeoffrey wrote:

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

In D90352#2361384, @tschuett wrote:

Could you add something similar to https://github.com/llvm/llvm-project/blob/master/llvm/utils/gn/README.rst?

• How to build something?
• How to use clang/llvm/mlir as an external dependency?

In D89758#2352745, @lhames wrote:

Hi Geoffrey,

Apologies for the delayed reply. Can you share details of the failure
you're seeing?

Regards,
Lang.

In D89142#2352061, @tschuett wrote:

How about keeping the Bazel files in-tree?

• more customers
• better visibility
• lower maintenance costs
• ...

Rebase on master again

Landing with failed builds. Looks like the same test is failing at HEAD: https://buildkite.com/llvm-project/llvm-master-build/builds/1281#df0e1b85-07a8-4199-8c41-79b6b38bd099

Update commit message

Hmmm seems this breaks lld on windows somehow... @lhames could you investigate a more principled fix? I think the approach of providing an alternative implementation if LLVM_ENABLE_THREADS is false would be consistent with other places in the file

Rebase on master

• Improve error message

In D89758#2340716, @hubert.reinterpretcast wrote:

We've been building with LLVM_ENABLE_THREADS=0 to avoid dependencies on the TLS support in the build environment. We may require assistance to restore our builds if these patches introduce a TLS dependency. @daltenty, fyi.

Note that I'm not familiar with this part of the codebase, so if there's a better fix (like we can add a guard to the std::promise usage) then I'm happy to do that instead. I'm just trying to maintain a LLVM_ENABLE_THREADS=0 build because of https://bugs.llvm.org/show_bug.cgi?id=44211.

Thanks!

In D89142#2323331, @thieta wrote:

In D89142#2322663, @mehdi_amini wrote:

Wow pretty neat to see llvmbuild being removed!

That said we are still depending on it (we generate Bazel build file from this and use this in many projects using MLIR like TensorFlow, XLA, IREE...).
We need to move away from it: can you ping me and/or @GMNGeoffrey when you get closer to have this in a landing state?

Could the generation of Bazel files be done by parsing the compile_commands.json maybe? that seems like a more "portable" approach since it's a pretty standard format these days.

Committed as https://github.com/llvm/llvm-project/commit/93db4a8ce6261dc36e233f5e0b60cfbb3ea1bd8f

Hey JFYI this created some unused variables. I fixed with https://reviews.llvm.org/D89022

• Fix flang

Whoops meant to upload a patch to an existing revision

Oh huh, I broke flang :-D I was just replacing in the MLIR directory

Thanks for improving error messages Mehdi!

Thanks for reviewing!

Drop NOLINTNEXTLINE

In D86892#2253328, @MaskRay wrote:

In D86892#2253324, @GMNGeoffrey wrote:

If y'all are good with this now (and think that others will not have objects), could one of you land it for me? I don't have commit access.

Will do:) You can drop NOLINTNEXTLINE(readability-identifier-naming). The issues are known and are everywhere... Just ignore them and don't bother annotating the code...

If y'all are good with this now (and think that others will not have objects), could one of you land it for me? I don't have commit access.

Use #else after throwing exception

Suppress pre-existing clang-tidy warnings

And one more typo...

Fix typo

Fix lint. Switch to explicit const char *

Thanks!