Stream: t-compiler/major changes

Topic: Change `ty::Const` to an integer tree repr compiler-team#323


triagebot (Jul 01 2020 at 08:49, on Zulip):

A new proposal has been announced: #323. It will be
announced at the next meeting to try and draw attention to it,
but usually MCPs are not discussed during triage meetings. If
you think this would benefit from discussion amongst the
team, consider proposing a design meeting.

lcnr (Jul 01 2020 at 09:06, on Zulip):

Nice :heart:

lcnr (Jul 01 2020 at 09:10, on Zulip):

Move parts from ConstKind to ConstValue, allowing things like (42, N) to be encoded, while currently const generic parameters can only be encoded if the constant is just a const generic parameter directly.

@oli Can you give an example where this is useful? I think we currently either have fully monomorphic ConstKinds and things like (42, N) are represented as ConstKind::Unevaluated. I didn't yet think too deeply about this, but are there some patterns that are hard/impossible without this possible future change?

oli (Jul 01 2020 at 09:12, on Zulip):

We may want to not represent them as ConstKind::Unevaluated. I don't really know :slight_smile: I am mainly mentioning it for completeness

oli (Jul 01 2020 at 09:12, on Zulip):

cc @varkor

lcnr (Jul 01 2020 at 09:13, on Zulip):

It might be required if we want to be able to unify (42, N) with (42, M) and infer N == M, which isn't possible rn but might be desirable in the future

bjorn3 (Jul 01 2020 at 09:17, on Zulip):

Duplicate ty::Const into ty::Const and mir::Const where the former becomes a tree with integer leaves and the latter loses all forms of “optimized” representation and just refers to the plain const allocation backing the constant.

That would mean that a mir::Const representing 42u8 suddenly gets an Allocation allocated instead of using ConstValue::Scalar, thus increasing the memory usage. Probably by a significant amount.

lcnr (Jul 01 2020 at 09:18, on Zulip):

Wouldn't 42u8 be just represented as ConstValue::Leaf(42u128)?

bjorn3 (Jul 01 2020 at 09:18, on Zulip):

That is for ty::Const, not for mir::Const.

oli (Jul 01 2020 at 09:21, on Zulip):

bjorn3 said:

Duplicate ty::Const into ty::Const and mir::Const where the former becomes a tree with integer leaves and the latter loses all forms of “optimized” representation and just refers to the plain const allocation backing the constant.

That would mean that a mir::Const representing 42u8 suddenly gets an Allocation allocated instead of using ConstValue::Scalar, thus increasing the memory usage. Probably by a significant amount.

This already happens, we just throw the reference to it away.

oli (Jul 01 2020 at 09:21, on Zulip):

const_eval_raw will give you an &'tcx Allocation to the constant

oli (Jul 01 2020 at 09:21, on Zulip):

const_eval_validated invokes const_eval_raw, thus causing this allocation to always be created

oli (Jul 01 2020 at 09:25, on Zulip):

so incremental caches aren't affected by this change, but it may have an effect on the size of the MIR encoded in metadata, since that currently indeed does not create an Allocation for integer constants. I'll make sure to do this change as its own PR and check the size of artifacts

RalfJ (Jul 06 2020 at 17:09, on Zulip):

A lossy conversion from mir::Const to ty::Const will be introduced. It will be used for pretty printing mir::Const during mir dumps. This way we only have to support pretty printing ty::Const, which is trivial to pretty print.

what do you plan to do for types not representable in ty::Const? After all, one key point here is that not all types fit ty::Cosnt -- but the consumers of ty::Const cannot properly support those types anyway.
examples include unions and raw pointers.

RalfJ (Jul 06 2020 at 17:10, on Zulip):

re: the concrete ConstValue, I wonder if Leaf should have a size information like Scalar::Raw does. it served us well there.

RalfJ (Jul 06 2020 at 17:13, on Zulip):

also could the hackmd describe how the queries will be organized (re: https://github.com/rust-lang/rust/issues/72396)?
my expectation is that there will be one query that everything goes through that returns a mir::Const after validating type-based invariants. and then there is a second query that returns a ty::Const but this can fail if the type of the const is not fit for integer trees. (or if the values does not match the type, which can still happen behind static indirections -- where validation has to stop to avoid query cycles.)

oli (Jul 07 2020 at 10:45, on Zulip):

I didn't add that part to the MCP as it's technically an orthogonal refactoring to move validation to the raw query

oli (Jul 07 2020 at 10:46, on Zulip):

RalfJ said:

re: the concrete ConstValue, I wonder if Leaf should have a size information like Scalar::Raw does. it served us well there.

I left it out on purpose, but we could keep it in initially to keep the change from the current system small

oli (Jul 07 2020 at 10:47, on Zulip):

Types not representable in ty::Const get printed via the Allocation printing

triagebot (Jul 08 2020 at 16:41, on Zulip):

@T-compiler: Proposal #323 has been seconded, and will be approved in 10 days if no objections are raised.

RalfJ (Jul 12 2020 at 10:52, on Zulip):

oli said:

RalfJ said:

re: the concrete ConstValue, I wonder if Leaf should have a size information like Scalar::Raw does. it served us well there.

I left it out on purpose, but we could keep it in initially to keep the change from the current system small

what is the reasoning for not doing size tracking any more?

oli (Jul 12 2020 at 11:29, on Zulip):

mostly memory usage, iirc this is the main reason we can't shrink ConstValue down any more, and considering that the most common value is usize and bool, it seems very wasteful. Maybe I should figure out some other representation where u128 is behind a box or sth. Anyway, let's keep it in and table the discussion until we actually try to remove the size field.

RalfJ (Jul 13 2020 at 07:04, on Zulip):

(FWIW since this is outside Miri I'd be okay to remove the size field if you want to go that route, we'd still get our sanity checks during evaluation. I just wanted to understand why it is worth dropping the safety net.)

eddyb (Jul 13 2020 at 07:13, on Zulip):

you could have u128 and i128 leaves, tbh

eddyb (Jul 13 2020 at 07:14, on Zulip):

so that the "abstract value" is represented, no encoding signed integers into u128

eddyb (Jul 13 2020 at 07:15, on Zulip):

(and presumably "byte stream" leaves for "..." and b"..." literals :P)

RalfJ (Jul 13 2020 at 07:27, on Zulip):

eddyb said:

so that the "abstract value" is represented, no encoding signed integers into u128

avoiding signedness issues would be awesome indeed

RalfJ (Jul 13 2020 at 07:27, on Zulip):

but if we haveu128/i128 that means we need a tag and that means we likely have enough space for the size

oli (Jul 13 2020 at 07:28, on Zulip):

eddyb means to make these not sub-enums of the Leaf variant, but just add multiple Leaf variants

RalfJ (Jul 13 2020 at 07:29, on Zulip):

sure

oli (Jul 13 2020 at 07:29, on Zulip):

though at that point you're right, we may be able to reuse the space in the tag, since the tag will do a bump to requiring another usize bytes

eddyb (Jul 13 2020 at 07:32, on Zulip):

and the "size" is just an u8, right?

RalfJ (Jul 13 2020 at 07:33, on Zulip):

yeah, until we have integers longer than 255 bytes^^

RalfJ (Jul 13 2020 at 07:33, on Zulip):

but even then we could just encode log2(size)

Last update: May 07 2021 at 06:15UTC