Set lto to 'fat' with autodiff enabled

[helldawg]

resolves #142796

This took a whole lot longer than I expected it to. A lot longer.

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rust-log-analyzer]

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

fmt check
Diff in /checkout/compiler/rustc_interface/src/interface.rs:483:
                 config.expanded_args,
             );
             // Forces lto="fat" if autodiff is enabled.
-   if sess.opts.unstable_opts.autodiff.contains(&config::AutoDiff::Enable) {
-    sess.opts.cg.lto = config::LtoCli::Fat;
-   }
+            if sess.opts.unstable_opts.autodiff.contains(&config::AutoDiff::Enable) {
+                sess.opts.cg.lto = config::LtoCli::Fat;
+            }
 
             codegen_backend.init(&sess);
 
fmt: checked 6369 files

[helldawg]

r? claim
r? compiler

[rustbot]

Failed to set assignee to claim: cannot assign: response: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/assignees#add-assignees-to-an-issue","status":"404"}

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[helldawg]

@rustbot label C-bug F-autodiff

[Urgau]

LTO is meant to be an optimization and therefor be optional. Why does autodiff needs it to be unconditionally enabled? Does that mean that autodiff just doesn't work without fat LTO?

[helldawg]

LTO is meant to be an optimization and therefor be optional. Why does autodiff needs it to be unconditionally enabled? Does that mean that autodiff just doesn't work without fat LTO?

Yeah, check out the linked issue. Using autodiff without setting lto="fat" in your Cargo.toml caused it to set derivatives to 0.0. When I tried it more recently, it didn't compile at all.

[ZuseZ4]

@Urgau Enzyme (the llvm plugin doing autodiff for us) needs access to the whole IR, so fat-lto is the simplest solution. That's just based on the chain rule from calculus, you need to see all function (and type) definitions which are used recursively/indirectly.

Technically, embed-bc should also work, but there's an issue around how we get the llvm-ir of the std library (IIRC) in the non-fat-lto case. Oli and I spend a few hours a while ago trying to figure that out and we think that some of the logic in rustc is wrong, but we both ran out of time. I'll get back to it at some point.

Relatedly, GPU code has the same requirement. #135024
There are people working on lowering it to some variant of thin-lto, but we're not fully there yet.

That being sad, it looks like that PR unfortunately raced with #146229, which I think is the better location to enable lto, since we already seem to check there for other reasons to enable lto.
Since the issue was open for a few months I didn't expect two people to find a solution simultaneously, sorry for that. Do you mind to instead make a similar PR to enable fat-lto for the GPU targets (to solve the tracking issue I linked above)? That way both of you can land one version of this code.

[helldawg]

@ZuseZ4 I don't mind at all, but it looks like it already does that?

In the amdgcn spec (rust/compiler/rustc_target/src/spec/targets/amdgcn_amd_amdhsa.rs):

// The LLVM backend does not support stack canaries for this target
            supports_stack_protector: false,

            // Force LTO, object linking does not yet work with amdgpu.
            requires_lto: true,   <=== This is set

            ..Default::default()
        },
    }
}

And I assume it would be used here (rust/compiler/rustc_session/src/session.rs):

/// Calculates the flavor of LTO to use for this compilation.
    pub fn lto(&self) -> config::Lto {
        // If our target has codegen requirements ignore the command line
        
        if self.target.requires_lto {    <======
            return config::Lto::Fat;
        }

[ZuseZ4]

cc @Flakebi Did you forget to check the box for fix requiring lto automatically? (see above)

[Flakebi]

cc @Flakebi Did you forget to check the box for fix requiring lto automatically? (see above)

When I tested ~2 weeks ago, it was still broken unfortunately, so the explicit lto=fat in Cargo.toml is still needed sometimes (I know the code quoted above sets it, so I don’t know where it’s broken, I didn’t get to investigate further).