Ignore mir tests on big-endian

[infinity0]

Other mir tests are already ignored on big-endian, e.g. const_allocation*.rs

[rust-highfive]

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

This seems to be a "significant" diff -- i.e., not just line numbers changing. Maybe a bug in how github is showing it or a rebase error?

[ecstatic-morse]

This is just line numbers changing AFAICT. Diffs of diffs are hard to parse for me also.

[oli-obk]

Things like these can be avoided by adding new comment lines in existing empty lines or just adding them at the end of the file. We have no policy on this though

[infinity0]

Yes this is just line numbers changing - the existing file didn't have an empty line to fit a comment into, and I thought putting it at the end of the file would be inconsistent with everything else.

[Mark-Simulacrum]

I assume these are failing on big endian platforms?

I think the const allocation ones are ignored because the output contains printed allocations (e.g.,

rust/src/test/mir-opt/const_allocation2.main.ConstProp.after.32bit.mir

Lines 31 to 33 in 738d4a7

	0x00 │ 00 00 00 00 __ __ __ __ ╾─alloc8──╼ 00 00 00 00 │ ....░░░░╾──╼....
	0x10 │ 00 00 00 00 __ __ __ __ ╾─alloc12─╼ 02 00 00 00 │ ....░░░░╾──╼....
	0x20 │ 01 00 00 00 2a 00 00 00 ╾─alloc19─╼ 03 00 00 00 │ ....*...╾──╼....

) which are not normalized to a particular endian-ness currently (probably no need to do so either).

But the tests here, aside from the one comment I left, don't seem to contain such allocations. I am a bit worried that we could accidentally mask some real difference in const prop or whatever on big endian systems. If we expect to just ignore all of the MIR tests then maybe doing that in compiletest makes more sense.

cc @oli-obk @ecstatic-morse (not sure who all to cc on const prop/MIR, would be good to get a team)

[ecstatic-morse]

cc @rust-lang/wg-mir-opt

[ecstatic-morse]

@infinity0 It's helpful to reviewers to give some motivation for a PR. I assume you're running on a big-endian system and that these tests are failing due to blessed MIR like:

+         _1 = const (4_u32, false);       // scope 0 at $DIR/return_place.rs:6:5: 6:10
+                                          // ty::Const
+                                          // + ty: (u32, bool)
+                                          // + val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [31], len: Size { raw: 8 } }, size: Size { raw: 8 }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } })
+                                          // mir::Constant
+                                          // + span: $DIR/return_place.rs:6:5: 6:10
+                                          // + literal: Const { ty: (u32, bool), val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [31], len: Size { raw: 8 } }, size: Size { raw: 8 }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) }

Do we not have any big-endian systems on CI? Normally I would request that we normalize MIR dumps across different systems, but this is a) kind of hard and b) not the precedent (as you note in the OP). I defer to Oli on this one.

r? @oli-obk

[oli-obk]

Instead of ignoring this test we should add tuples and arrays to the "don't verbosely print the constant" list if all tuple fields or the array element type are on the "don't verbosely print constants of this type" list.

[oli-obk]

This one is odd, so I think it's ok to ignore here

[oli-obk]

So imo only src/test/mir-opt/const_prop/mutable_variable_no_prop.rs should be ignored, everything else can be fixed

[infinity0]

we should add tuples and arrays to the "don't verbosely print the constant" list if all tuple fields or the array element type are on the "don't verbosely print constants of this type" list.

Shall I file an issue for this then? I'm not sufficiently involved with MIR to make this change myself. In the meantime do you want to keep the tests failing?

[oli-obk]

Oh, this is not related to MIR, just pretty printing. All you have to do is to pull out

rust/compiler/rustc_mir/src/util/pretty.rs

Lines 432 to 436 in 5ded394

	match ty.kind() {
	ty::Int(_) | ty::Uint(_) | ty::Bool | ty::Char | ty::Float(_) => {}
	// Unit type
	ty::Tuple(tys) if tys.is_empty() => {}
	ty::FnDef(..) => {}

into a separate function that returns true if the verbose printing should be used and then make it recurse on ty::Tuple and ty::Array elements

The only gotcha is that you need to rerun tests separately for 64bit and 32bit via --target i686-unknown-linux-gnu.

[crlf0710]

@infinity0 Ping from triage! Any updates on this?

[JohnCSimon]

@infinity0 Pinging again from triage! Any updates on this?

[infinity0]

Sorry, I don't have the time to do the alternative. But the underlying problem exists and should be fixed.

[oli-obk]

I'll open an issue and see if someone picks it up. Thanks for bringing up the issue!