Skip to content

bootstrap should rebuild LLVM when it changes #111893

Closed
#118187
Closed
bootstrap should rebuild LLVM when it changes#111893
#118187
Assignees
onur-ozkan
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Description

@jyn514
jyn514
opened on May 24, 2023

I tried this code:

; ./configure --set llvm.download-ci-llvm=false
; x build --stage 0 rustc_llvm
; (cd src/llvm_project && git apply)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index dd86144d16..43395b1ecd 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -153,6 +153,8 @@ static void dumpStringOffsetsSection(raw_ostream &OS, DIDumpOptions DumpOpts,
                                      StringRef StringSection,
                                      DWARFContext::unit_iterator_range Units,
                                      bool LittleEndian) {
+  OS << "debug: dumpStringOffsetsSection" << "\n";
+
   auto Contributions = collectContributionData(Units);
   DWARFDataExtractor StrOffsetExt(Obj, StringOffsetsSection, LittleEndian, 0);
   DataExtractor StrData(StringSection, LittleEndian, 0);
; x build --stage 0 rustc_llvm

I expected to see this happen: bootstrap rebuilds LLVM, since it's changed.

Instead, this happened: bootstrap does nothing.

The problem is that we 're using a stamp file and assume that if the commit hasn't changed then none of the files have changed either:

rust/src/bootstrap/llvm.rs

Lines 108 to 123 in 95e8b6a

let stamp = out_dir.join("llvm-finished-building");
let stamp = HashStamp::new(stamp, builder.in_tree_llvm_info.sha());
if stamp.is_done() {
if stamp.hash.is_none() {
builder.info(
"Could not determine the LLVM submodule commit hash. \
Assuming that an LLVM rebuild is not necessary.",
);
builder.info(&format!(
"To force LLVM to rebuild, remove the file `{}`",
stamp.path.display()
));
}
return Ok(res);
}

I am not sure why we do that? Are we assuming that CMake rebuilds too frequently? That seems like something we should fix upstream ...

cc @rust-lang/wg-llvm, I think this would make your lives easier.

Meta

HEAD is branched from ba6f5e3.

Activity

  1. jyn514
    added
    C-bugCategory: This is a bug.
    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
    T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
    on May 24, 2023

  2. jyn514 commented on May 24, 2023

    @jyn514
    jyn514
    on May 24, 2023
    MemberAuthor

    hmm, #64156 looks related. I am not sure why that would be the correct behavior? If CFLAGS has changed we should rebuild; I would expect distros to make sure CFLAGS matches between invocations of bootstrap.

  3. jyn514 commented on May 24, 2023

    @jyn514
    jyn514
    on May 24, 2023
    MemberAuthor

    The stamp checking for the "normal" case, when not building from a source tarball, has been there since the initial commit of bootstrap: 046e687#diff-ddd5cbd996a9fbebe15be7bb2d9254ea4c1336d6c2e47a5da2a76719df572c01R30-R38

  4. cuviper commented on May 24, 2023

    @cuviper
    cuviper
    on May 24, 2023
    Member

    Are you trying to address the git "dirty" case, or the non-git case, or both?

  5. jyn514 commented on May 24, 2023

    @jyn514
    jyn514
    on May 24, 2023
    MemberAuthor

    Both, but primarily the git case since that's how most people are developing I think.

  6. cuviper commented on May 24, 2023

    @cuviper
    cuviper
    on May 24, 2023
    Member

    Ok, I agree the git case is where folks may actually be making changes, and I still think it's ok not to automatically "notice" changes in the non-git case.

    LLVM is big enough that checking for dirty git may not be cheap though. On my Linux system with NVME, it takes ~0.5s after dropping page caches, and ~0.15s cached. On systems with slower disks, virus scanners, etc., it may be a lot worse.

  7. nikic commented on May 24, 2023

    @nikic
    nikic
    on May 24, 2023
    Contributor

    Yeah, I don't think this is particularly important. The main issue with making changes to LLVM is #109070, which prevents the change from being picked up even if you do commit it.

  8. onur-ozkan
    self-assigned this
    on Nov 22, 2023
  9. compiler-errors
    added a commit that references this issue on Nov 24, 2023

    Rollup merge of rust-lang#118187 - onur-ozkan:recompile-llvm-on-chang…

    7f974b9
  10. bors
    closed this as completedin 7f974b9on Nov 24, 2023
  11. rust-timer
    added a commit that references this issue on Nov 24, 2023

    Unrolled build for rust-lang#118187

    0156fa1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions