This is an archive of the discontinued LLVM Phabricator instance.

[lldb] add a missing dependency on intrinsics_gen
ClosedPublic

Authored by rmaz on Nov 5 2020, 8:45 AM.

Details

Summary

Sometimes builds will fail with errors like:

In file included from /build/external/llvm-project/lldb/source/Symbol/SwiftASTContext.cpp:52:
In file included from /build/external/swift/include/swift/IRGen/Linking.h:22:
In file included from /build/external/swift/include/swift/SIL/SILFunction.h:24:
In file included from /build/external/swift/include/swift/SIL/SILBasicBlock.h:23:
In file included from /build/external/swift/include/swift/SIL/SILInstruction.h:21:
In file included from /build/external/swift/include/swift/AST/Builtins.h:24:
**/build/external/llvm-project/llvm/include/llvm/IR/Attributes.h:74:14: fatal error: 'llvm/IR/Attributes.inc' file not found**
#include "llvm/IR/Attributes.inc"
**^~~~~~~~~~~~~~~~~~~~~~~~**

This change ensures the Attributes.inc file is generated before building SwiftASTContext.cpp.

Diff Detail

Event Timeline

rmaz created this revision.Nov 5 2020, 8:45 AM
rmaz requested review of this revision.Nov 5 2020, 8:45 AM
kastiglione added a subscriber: kastiglione.
kastiglione added inline comments.
lldb/source/Symbol/CMakeLists.txt
-9–-7

Looks like D83454 tried to do away with tablegen_deps, and instead have targets depend directly on intrinsics_gen. Should that be followed here too.

rmaz updated this revision to Diff 303304.Nov 5 2020, 5:25 PM

Update to use DEPENDS

rmaz marked an inline comment as done.Nov 5 2020, 5:26 PM
rmaz added inline comments.
lldb/source/Symbol/CMakeLists.txt
-9–-7

Good suggestion, updated to match the changes in D83454

rmaz updated this revision to Diff 303305.Nov 5 2020, 5:28 PM
rmaz marked an inline comment as done.
JDevlieghere accepted this revision.Nov 5 2020, 8:33 PM

LGTM

lldb/source/Symbol/CMakeLists.txt
43

nit: indentation

This revision is now accepted and ready to land.Nov 5 2020, 8:33 PM
kastiglione accepted this revision.Nov 6 2020, 8:16 AM
rmaz updated this revision to Diff 303468.Nov 6 2020, 8:47 AM

indent DEPENDS

rmaz marked an inline comment as done.Nov 6 2020, 8:47 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 6 2020, 10:19 AM

Just to clarify: This only a dependency in the downstream Swift branch?

labath added a subscriber: labath.Nov 8 2020, 11:58 PM

Just to clarify: This only a dependency in the downstream Swift branch?

Judging by the error message, this dependency is not correct even in the swift fork. lldbSymbol does not have a direct dependency on llvmIR. It has a dependency on swift (through SwiftASTContext), and swift (through AST/Builtins.h) has a dependency on the generated headers.

So, the right fix should be to add a lldbSymbol->Swift dependency (if one isn't there already), and then a swift->LLVMIR dep.

Unless there are other, direct dependencies (include paths) from lldbSymbol to LLVMIR. However, I don't see any in the llvm repo, so it still sounds like something swifty...

Ping!

I'd still like to hear an explanation on how is this patch relevant (in this repository).

Per our code review policy, the responsibility for a patch does not end the moment the patch is committed :

Post-commit review is encouraged, and can be accomplished using any of the tools detailed below. There is a strong expectation that authors respond promptly to post-commit feedback and address it. Failure to do so is cause for the patch to be reverted.
rmaz added a comment.Nov 19 2020, 9:15 AM

I'd still like to hear an explanation on how is this patch relevant (in this repository).

Apologies, missed the last notification.

Judging by the error message, this dependency is not correct even in the swift fork. lldbSymbol does not have a direct dependency on llvmIR. It has a dependency on swift (through SwiftASTContext), and swift (through AST/Builtins.h) has a dependency on the generated headers.

Correct.

So, the right fix should be to add a lldbSymbol->Swift dependency (if one isn't there already), and then a swift->LLVMIR dep.

This sounds reasonable, the Swift build script seems to enforce a similar compilation order of:

  1. LLVM & Clang
  2. Swift
  3. LLDB

There are some existing dependencies on Swift libs in standalone mode, let me take a look at this approach instead.