emit "align 1" metadata on loads/stores of packed structs

[arielb1]

According to the LLVM reference:

A value of 0 or an omitted align argument means that the operation has
the ABI alignment for the target.

So loads/stores of fields of packed structs need to have their align set
to 1. Implement that by tracking the alignment of LvalueRefs.

Fixes #39376.

r? @eddyb

[eddyb]

Are you going to do something about this?

[arielb1]

This is hard to fix without a refactor.

[eddyb]

LGTM, except for that one FIXME. f? @Aatch, r=me if no concerns remain.

[arielb1]

And the FIXME turned out to be unneeded - ABI indirect arguments are always aligned.

@bors r=eddyb

[bors]

📌 Commit 56591f6 has been approved by eddyb

[Aatch]

@bors r=eddyb

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Add i686-unknown-netbsd target #39426

[bors]

📌 Commit 56591f6 has been approved by eddyb

[arielb1]

@bors r=eddyb

[bors]

📌 Commit 67789c4 has been approved by eddyb

[arielb1]

rebased

@bors r=eddyb

[bors]

📌 Commit d71988a has been approved by eddyb

[binarycrusader]

Unfortunately, this patch doesn't seem to resolve the issue fully. See #39376 for an example test case that appears to still fail.

Note that brute force hack patch also mentioned in the bug there that does resolve the issue for the test case shown.

[binarycrusader]

This is seen in the RUST_LOG=4 output when building the referenced example:

DEBUG:rustc_trans::mir::operand: trans_operand(operand=_15)
DEBUG:rustc_trans::mir::operand: trans_consume(lvalue=_15)
DEBUG:rustc_trans::mir::lvalue: trans_lvalue(lvalue=(*(*_4))[_15]) => LvalueRef { llval: 0x154f0e0, llextra: 0x0, ty: Ty { ty: Unaligned<u32> }, alignment: AbiAligned }
DEBUG:rustc_trans::mir::operand: trans_load: (%"Unaligned<u32>"*:  %26 = getelementptr inbounds %"Unaligned<u32>", %"Unaligned<u32>"* %23, i64 %13, !dbg !85) @ Unaligned<u32>
DEBUG:rustc_trans::mir::operand: store_operand: operand=OperandRef(Immediate((%"Unaligned<u32>" = type <{ i32 }>:  %27 = load %"Unaligned<u32>", %"Unaligned<u32>"* %26, !dbg !85)) @ Unaligned<u32>), align=None
DEBUG:rustc_trans::builder: Store (%"Unaligned<u32>" = type <{ i32 }>:  %27 = load %"Unaligned<u32>", %"Unaligned<u32>"* %26, !dbg !85) -> (%"Unaligned<u32>"*:  %_14 = alloca %"Unaligned<u32>")

[arielb1]

@binarycrusader

I compile your example using your patch - that's it,

use std::slice;
use std::u32;

#[repr(packed)]
#[derive(Copy, Clone)]
struct Unaligned<T>(T);

impl<T> Unaligned<T> {
    fn get(self) -> T { self.0 }
}

fn bytes_to_words(b: &[u8]) -> &[Unaligned<u32>] {
    unsafe { slice::from_raw_parts(b.as_ptr() as *const Unaligned<u32>, b.len() / 4) }
}

fn main() {
        let values = String::from("fedcba98765432100123456789abcdef");
        let bytes = values.as_bytes();
        let mut words = &bytes_to_words(&bytes[1..]);
        let index = 1;
        let position = u32::from_le(words[index].get());
}

and it works (emits align metadata on the necessary loads).

Is there some other example you are talking about?

Immediate((%"Unaligned" = type <{ i32 }>: %27 = load %"Unaligned", %"Unaligned"* %26, !dbg !85)) @ Unaligned

We should not be having this (immediate Unaligned<u32>) in code. Are you using some old version of rustc with my patch applied?

[bors]

⌛ Testing commit d71988a with merge b0e46f0...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing b0e46f0 to master...

[binarycrusader]

Would someone be willing to beta-nominate this? This patch has been used successfully against 1.16 beta for almost two weeks and resolved the last code generation issue we encountered on sparc (and I suspect would help other architectures too). I believe it applies cleanly without any changes to current beta tip.