Stream: t-compiler

Topic: change rustc_args_required_const to per arg attribute


jakevossen5 (Mar 21 2020 at 17:05, on Zulip):

Hey all,

I am currently working on 69282 which changes #[rustc_args_required_const] to a per argument attribute (implemented here).

I was talking to @Vadim Petrochenkov and he suggested reaching out to you about the best way to encode a argument attribute in the MIR. He suggested maybe a boolean flag specifically for this rustc_required_const instead of encoding all attributes.

varkor (Mar 21 2020 at 23:48, on Zulip):

why do we want this to affect MIR?

varkor (Mar 21 2020 at 23:49, on Zulip):

I was under the impression rustc_args_required_const was always a temporary hack — we can achieve the same thing with const generics, so we shouldn't special-case too much to support it

eddyb (Mar 22 2020 at 01:07, on Zulip):

@jakevossen5 You need to access this from outside the function, not from the MIR body (which is the "inside" of the function, so the speak)

eddyb (Mar 22 2020 at 01:09, on Zulip):

@jakevossen5 the most important part is how you can detect this cross-crate. locally you can just look at the HIR, but cross-crate is the hard part, and if you do that right then you'll end up with a query which you can implement locally too (and encoding the cross-crate data would just use the query)

eddyb (Mar 22 2020 at 01:09, on Zulip):

probably a decent way to do it would be like this https://github.com/rust-lang/rust/blob/master/src/librustc_metadata/rmeta/mod.rs#L330

eddyb (Mar 22 2020 at 01:12, on Zulip):

left this comment https://github.com/rust-lang/rust/issues/69282#issuecomment-602130836

bjorn3 (Mar 22 2020 at 10:31, on Zulip):

How hard would it be to turn rustc_required_const into real const generics when desugaring to HIR? That would make bjorn3/rustc_codegen_cranelift#666 much easier to implement.

Amanieu (Mar 22 2020 at 10:53, on Zulip):

FYI I use the same mechanism as rustc_required_const for const operands to inline assembly.

Amanieu (Mar 22 2020 at 10:55, on Zulip):

... which means that my code is currently broken since it also assumes that the MIR resolves to a constant...

Amanieu (Mar 22 2020 at 10:59, on Zulip):

@bjorn3 Have you tried calling eval_mir_constant? https://github.com/rust-lang/rust/pull/69171/files#diff-a5b7af844d26b58c2c9537372b100c90R938

bjorn3 (Mar 22 2020 at 11:05, on Zulip):

Yes, I am using eval_mir_constant. The problem is that for example _mm256_extract_epi64 is a single function, that is codegened a single time without being specialized on the imm8 argument. However the simd_extract intrinsic needs to know the value of imm8 at during codegen. With cg_llvm this is not a problem, as it only needs to have a known value after inlining and other optimizations, but clif ir treats it as a real immediate, so you can't pass a value that will be known after optimizations.

Amanieu (Mar 22 2020 at 11:12, on Zulip):

Ah I see. Does #[inline(always)] force MIR inlining?

Amanieu (Mar 22 2020 at 11:13, on Zulip):

(deleted)

bjorn3 (Mar 22 2020 at 11:15, on Zulip):

No, unfortunately not.

  1. MIR inlining is only enabled at mir-opt-level>=2
  2. MIR inlining is skipped in many cases to prevent potential cycles during inlining
  3. MIR inlining has some bugs, so I can't enable it by default in cg_clif
eddyb (Mar 22 2020 at 12:19, on Zulip):

wait how does that even work

eddyb (Mar 22 2020 at 12:19, on Zulip):

is simd_extract being used by _mm256_extract_epi64 with only the latter having #[rustc_args_required_const]?

eddyb (Mar 22 2020 at 12:20, on Zulip):

because that's scary, intrinsics should be correctly annotated

eddyb (Mar 22 2020 at 12:20, on Zulip):

IIRC I was told this wouldn't be allowed to happen

eddyb (Mar 22 2020 at 12:21, on Zulip):

uhhhh ohhhhhhhh https://doc.rust-lang.org/src/core/up/stdarch/crates/core_arch/src/x86_64/avx2.rs.html#28-30

eddyb (Mar 22 2020 at 12:21, on Zulip):

so we stabilized functions like this?!

eddyb (Mar 22 2020 at 12:22, on Zulip):

@Amanieu @bjorn3 the only correct thing to do, IMO, would be to force #[rustc_args_required_const(...)] to be applied to an intrinsic, and to make all of the users of the attribute intrinsics

eddyb (Mar 22 2020 at 12:23, on Zulip):

note that your inline assembly should be fine, and so should intrinsics

eddyb (Mar 22 2020 at 12:23, on Zulip):

it's only functions where it doesn't make much sense

eddyb (Mar 22 2020 at 12:23, on Zulip):

this is all because we have forsaken platform-intrinsics

eddyb (Mar 22 2020 at 12:23, on Zulip):

which was for silly concerns such as optimizing for adding intrinsics one by one

eddyb (Mar 22 2020 at 12:24, on Zulip):

as opposed to treating it like a data entry thing

Amanieu (Mar 22 2020 at 12:24, on Zulip):

Maybe once we have const generics we could make #[rustc_args_required_const] just desugar to const generics.

eddyb (Mar 22 2020 at 12:24, on Zulip):

try not to assume that, though

eddyb (Mar 22 2020 at 12:25, on Zulip):

intrinsics and inline assembly are fine, because they're builtin so we can attribute arbitrary semantics to them

eddyb (Mar 22 2020 at 12:27, on Zulip):

anyway I have to deal with another mess atm, and then probably just sleep

Amanieu (Mar 22 2020 at 12:30, on Zulip):

@bjorn3 How about forcing MIR inlining just for functions marked #[rustc_args_required_const(...)]? Since this attribute is only used in stdarch, we should be able to avoid all the existing issues with MIR inlining.

Amanieu (Mar 22 2020 at 12:37, on Zulip):

Either than or monomorphizing such functions as you would with const generics. I don't really see any other solutions.

eddyb (Mar 22 2020 at 12:37, on Zulip):

the real solution is to get rid of it on functions with bodies

eddyb (Mar 22 2020 at 12:39, on Zulip):

how come stdarch is still not yet in tree? what a disaster

eddyb (Mar 22 2020 at 12:44, on Zulip):

anyway, opened https://github.com/rust-lang/rust/issues/70271

jakevossen5 (Mar 23 2020 at 13:39, on Zulip):

Thank you, I will look into this!

jakevossen5 (Mar 23 2020 at 13:40, on Zulip):

Does this change how I should do my implementation?

Last update: May 29 2020 at 18:00UTC