Unaligned atomic load/store

Hi,

While working on the AVR target, I noticed a problem with alignment and the various atomic operations. AVR is an 8-bit platform, and all data is unaligned (or 8-bit aligned if you will). Here's a small piece of LLVM IR exhibiting this (emitted by the Rust compiler):

target datalayout = "e-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8"
target triple = "avr-unknown-unknown"

%"atomic::AtomicI16" = type { %"core::cell::UnsafeCell<i16>", [0 x i8] }
%"core::cell::UnsafeCell<i16>" = type { i16, [0 x i8] }

define void @foo(%"atomic::AtomicI16"*) {
start:
   %_15 = alloca i16, align 1
   %1 = getelementptr inbounds %"atomic::AtomicI16", %"atomic::AtomicI16"* %0, i16 0, i32 0, i32 0
   %2 = load atomic i16, i16* %1 seq_cst, align 1
   ret void
}

This trips up the following assertion in CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:

   if (I.getAlignment() < VT.getSizeInBits() / 8)
     report_fatal_error("Cannot generate unaligned atomic load");

I've tried commenting out the check and llc finishes, generating not-obviously-wrong machine code, so there doesn't seem to be anything further downstream breaking because of this.

So my questions are:

* What is the purpose of this assertion?
* What is the right way to handle this situation in an unaligned target?

Thanks,
   Gergo

Existing targets so far simply don't support unaliged atomic ops. That's
why it hasn't been refactored into a target information hook.

Joerg

So does that mean that this assertion is simply being over-defensive in anticipation of target-specific code further downstream failing on non-aligned atomic operations? It seems like a much better place for an assertion like this would be near the target-specific code (if any) that needs this precondition.

Can you recommend a way forward? Should I add some new method that tells the non-target-specific code the alignment requirements of the current target?

Not to put words in Joerg’s mouth, but he kind of alluded to a possible way forward.

You might want to define a target hook (say in Target Lowering) for say allowsUnalignedAtomicMemOps() that would return false on all targets but yours. Of course, you should definitely investigate whether there is a reason for this diagnostic (i.e. whether some other passes require natural alignment).

Another thing I find useful is using svn blame on the file in question to see what revision the diagnostic was added in. Chances are the commit message or the associated review will have some info on why it was needed.