- 
                Notifications
    You must be signed in to change notification settings 
- Fork 351
[CAS] gmodule support for caching build #11026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
2bbc405    to
    075be6d      
    Compare
  
    075be6d    to
    2a1ffca      
    Compare
  
    2a1ffca    to
    d5d9a15      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would potentially fit better into llvm-project/lldb/packages/Python/lldbsuite/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a compiler launcher type of script and requires a just built clang (because it needs clang-scan-deps next to it). I don't what is a good way to hack into lldbsuite or driven from makefile. Let me know if you have any good idea.
d5d9a15    to
    0173e49      
    Compare
  
    8063aee    to
    512a886      
    Compare
  
    512a886    to
    cb036bf      
    Compare
  
    | @adrian-prantl @benlangmuir Ping for review | 
cb036bf    to
    4ae6ba2      
    Compare
  
    | @swift-ci please test llvm | 
4ae6ba2    to
    5e96cdf      
    Compare
  
    | @swift-ci please test llvm | 
479c33e    to
    5c6f640      
    Compare
  
    | @swift-ci please test llvm | 
11bba2c    to
    55c97e3      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the clang changes. Someone else should review the LLDB and dsymutil code.
| /// Check debug info is correct. | ||
| // RUN: %clang %t/tu.o -o %t/a.out | ||
| // RUN: dsymutil -cas %t/cas %t/a.out -o %t/a.dSYM 2>&1 | FileCheck %s --check-prefix=WARN --allow-empty | ||
| // RUN: dsymutil %t/a.out -o %t/a2.dSYM 2>&1 | FileCheck %s --check-prefix=WARN --allow-empty | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this being dsymutil and not %dsymutil mean we might pick up the system one? Since there is also a dsymutil aspect to this, could that be problematic in CI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tools are always in the path.
NOTE: this is not the only test that calls dsymutil in clang tests and dsymutil wasn't even a build dependency for tests (added in this PR).
|  | ||
| def cas : Separate<["-", "--"], "cas">, | ||
| MetaVarName<"<path>">, | ||
| HelpText<"Specify CAS directory to load CAS references from.">, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this imply that there can only be one CAS per build?
Is that realistic? What if people use static archives and combine objects from different projects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it has to be one CAS per build. It is a requirement for module imports and there is no way you can configure otherwise.
If you have multiple builds and uses some products from other build, the second CAS has to ingest everything from previous builds.
| WithColor::note() << "create CAS using configuration: " << Config->first | ||
| << "'\n"; | ||
| return DB->first; | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to find a way to mock this up in a dsymutil-only test that doesn't depend on clang.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you have good ways to do it. It is very hard to do it since it needs to construct both object files (which can be committed) but also module files and CAS contents.
| // START CAS | ||
| FileSpec ModuleListProperties::GetCASOnDiskPath() const { | ||
| const uint32_t idx = ePropertyCASOnDiskPath; | ||
| return GetPropertyAtIndexAs<FileSpec>(idx, {}); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would these settings be communicated to LLDB in the typical use-case?
What if a debug session contains multiple dylibs built against different CAS storage? (I.e., in each project's DerivedData/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See ConfigureCASStorage().
This option provides a global overwrite. If no global setting available, it will search for a CAS configuration from the path of the debugging binary upwards to find a CAS configuration file.
Teach clang to encode CASID as splitDwarfFilename for gmodule when clang caching is enabled. This allows the outputs from compiler do not contain paths to the clang module files, thus allows distributed caching without the need of a unified clang module cache directory path.
Teach dsymutil how to use CAS and how to load clang modules from CAS when building dSYM when gmodule is used.
Teach lldb to load clang modules when gmodule + clang caching is used.
Teach SwiftASTContext to load clang module dependencies from CAS instead of FileSystem.
55c97e3    to
    a383255      
    Compare
  
    | Create PR on stable branch (so test is available) if you like to review over there: #11714 | 
Prototype for gmodule support by switching
splitDwarfreferences to CASIDs of the module/PCHAlso teach dsymutil to support reading from CAS as a prototype for lldb support.