Equal CTFE function pointers are unequal after codegen

[tmiasko]

$ cat a.rs 
pub static F: fn() = f::<u32>;
pub fn f<T>() {}
$ cat b.rs 
static F: fn() = a::F;
fn main() {
    assert_eq!(a::F, crate::F);
}
$ rustc --edition=2018 -O --crate-type=lib a.rs 
$ rustc --edition=2018 -O --crate-type=bin b.rs -L. --extern a
$ ./b
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0x561f15182780`,
 right: `0x561f15182660`'

The general issue is similar to #79738. In this case a function pointer is represented with GlobalAlloc::Function(Instance<'tcx>), which does not uniquely identify function after codegen.

cc @oli-obk

@rustbot modify labels: +A-codegen +A-const-eval

[RalfJung]

Hm, yeah this looks like #79738 -- but #79738 is more clear since function pointers additionally have a very unstable notion of equality.

@tmiasko do you think this is worth tracking separately from #79738?

[tmiasko]

With functions being anonymous allocations being duplicated? Sure, we can close this favour of #79738.

[tmiasko]

Reopening since the fix for #79738, is not sufficient to resolve this issue.

[RalfJung]

I think that's exactly what I talked about here, right? The "value" of the static is some AllocId aka some Instance, but actually the same Instance can turn into different function pointers when used in different crates.

But, is there even a way to tell LLVM that b::F want to point to the instance of f::<u32> that was generated inside a?

[tmiasko]

Yes, it is possible to refer to an instance from a specific crate. This is how share-generics mode is implemented. Presumably, in this case we would like to identify specific instances that would need to be exported, as opposed to exporting all of them.

[RalfJung]

Yes, it is possible to refer to an instance from a specific crate.

How are these even identified inside the compiler? An Instance just refers to any copy of the given instance of that function, right?

Presumably, in this case we would like to identify specific instances that would need to be exported, as opposed to exporting all of them.

So the "instance in the other crate" that we want to refer to needs to be exported before we can reference it?

Yes we'd only want to export those that show up in the value of some static or const. The monomorphization collector already walks all of them to ensure we create the right mono-items:

rust/compiler/rustc_monomorphize/src/collector.rs

Lines 1343 to 1348 in 339f4be

	GlobalAlloc::Function(fn_instance) => {
	if should_codegen_locally(tcx, fn_instance) {
	trace!("collecting {:?} with {:#?}", alloc_id, fn_instance);
	output.push(create_fn_mono_item(tcx, fn_instance, DUMMY_SP));
	}
	}

[oli-obk]

We could add a new GlobalAlloc kind that we use in statics to encode function pointers, and then use that to remember which crate a function is from and to always codegen it in the crate of the static if it isn't already codegenned in a dependency. That GlobalAlloc kind will be preserved cross crate and we then need to always refer to the one from the static's crate instead of relying on should_codegen_locally

[RalfJung]

Why would we not do that for our existing GlobalAlloc::Function?

[oli-obk]

Not sure how we'd do that with constants considering we don't know what function pointers they'll contain until we evaluate them downstream (at least if the constant is generic, we could start caching nongeneric or polymorphized constants)

[RalfJung]

GlobalAlloc::Function is never generic I think?

[oli-obk]

Could be with -Zpolymorphize, but I think you're right and we can just always do this, even if it's completely without an effect for constants.

Basically during interning, we also reintern AllocIds that refer to functions, and give them extra information about the crate that they were interned in.

[RalfJung]

With -Zpolymorphize, GlobalAlloc::Function should still be "as monomorphic as" the stuff we actually codegen.

IOW, GlobalAlloc::Function is an Instance and that should always be a single well-defined function. We just currently generate many copies of the same function under certain conditions, so we need some way to refer to a particular copy of that function and need to ensure this is reflected in the GlobalAlloc. As a starting point, this should probably be the copy used by whatever the current crate is when we actually compute the value.