Cross-crate global constants aren't being inlined

[alexcrichton]

I've got a project which uses extra::bitv::Bitv heavily, and I was looking at some time profiles of it recently. Currently the hottest function is bitv::Bitv::get, but if you look at the actual assembly heat map, there are two "hot" instructions, both of which are div operations.

Now this is very odd, because the only division/remainder done in bitv::Bitv::get is by uint::bits, a power-of-two constant. Hence, I would expect that these operations get optimized to shifts and ands.

I tried to reproduce this with a smaller test case, but LLVM's optimizations ended up thwarting me. I ended up generating the ll file for my project (282k lines of assembly), and found the definition of the get function. It appears that because it's marked #[inline], we inline it into the current crate. Because the division is done by the path uint::bits, this is translated to the load of a global constant before then passed to the udiv instruction for llvm.

Apparently, even though we inlined the entire function into another crate, we did not inline the constant. This global (uint::generated::bits) is then reflected in the LLVM module as:

@"...uint::generated::bits..." = external global i64

Which means that LLVM can no longer optimize based on the values of globals.

So my question is, is this something that we intend to support for optimizations? In a specific case like bitv perhaps we could remove the #[inline] annotation, but that's a bit of a shame. I'm not entirely sure that we can inline the values of global constants because that's only possible if the addresses are insignificant (which they're not by default right now).

In theory the values of constants could be inlined, but not much beyond immediates (constants with addresses to other constants may get tricky).

I'm not entirely sure what the best course of action to take here, but I wanted to see what others thought about this. The answer may just be removing #[inline] from bitv with a comment explaining why it can't be marked as such. This may have repercussions elsewhere though...

[alexcrichton]

cc @thestinger

[thestinger]

You can always expose the values of globals with the original symbol and use available_externally to avoid emitting a new symbol for them and hitting a conflict. It's only an ABI issue, never a correctness one related to addresses.

[alexcrichton]

Hmm, with that in mind I think that #8185 may have caused this regression.