Loading/storing an 8 aligned i128 uses two `movq` instead of a single `movups`

[LingMan]

Consider this example:

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @foo(ptr noalias %slice.0, ptr noalias %value) unnamed_addr {
start:
  %0 = load i128, ptr %value, align 8
  store i128 %0, ptr %slice.0, align 16
  ret void
}

Godbolt: https://llc.godbolt.org/z/fMMPbacGx

Expected asm output:

foo:                                    # @foo
        movups  xmm0, xmmword ptr [rsi]
        movaps  xmmword ptr [rdi], xmm0
        ret

Actual asm output:

foo:                                    # @foo
        mov     rax, qword ptr [rsi]
        mov     rcx, qword ptr [rsi + 8]
        mov     qword ptr [rdi + 8], rcx
        mov     qword ptr [rdi], rax
        ret

Replacing the two occurrences of i128 with <2 x i64> gives the expected output instead, so the issue appears to be specific to i128. Alternatively, changing the align 8 to align 16 causes LLVM to correctly output movaps instructions.

Real world background:

I noticed that filling a slice of u128 in Rust doesn't optimize as well as filling a slice of [u64; 2].

//type T = u128;
type T = [u64; 2];
pub fn foo(slice: &mut [T], value: &T) {
    slice.fill(*value)
}

[llvmbot]

@llvm/issue-subscribers-backend-x86

Author: None (LingMan)

Consider this example: ```llvm target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu"

define void @foo(ptr noalias %slice.0, ptr noalias %value) unnamed_addr {
start:
%0 = load i128, ptr %value, align 8
store i128 %0, ptr %slice.0, align 16
ret void
}

Godbolt: https://llc.godbolt.org/z/fMMPbacGx

Expected asm output:
```asm
foo:                                    # @<!-- -->foo
        movups  xmm0, xmmword ptr [rsi]
        movaps  xmmword ptr [rdi], xmm0
        ret

Actual asm output:

foo:                                    # @<!-- -->foo
        mov     rax, qword ptr [rsi]
        mov     rcx, qword ptr [rsi + 8]
        mov     qword ptr [rdi + 8], rcx
        mov     qword ptr [rdi], rax
        ret

Replacing the two occurrences of i128 with <2 x i64> gives the expected output instead, so the issue appears to be specific to i128. Alternatively, changing the align 8 to align 16 causes LLVM to correctly output movaps instructions.

Real world background:

I noticed that filling a slice of u128 in Rust doesn't optimize as well as filling a slice of [u64; 2].

//type T = u128;
type T = [u64; 2];
pub fn foo(slice: &mut [T], value: &T) {
    slice.fill(*value)
}

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. In the comments of the issue, request for it to be assigned to you.
2. Fix the issue locally.
3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a Git commit.
5. Run git clang-format HEAD~1 to format your changes.
6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: None (LingMan)

Consider this example: ```llvm target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu"

define void @foo(ptr noalias %slice.0, ptr noalias %value) unnamed_addr {
start:
%0 = load i128, ptr %value, align 8
store i128 %0, ptr %slice.0, align 16
ret void
}

Godbolt: https://llc.godbolt.org/z/fMMPbacGx

Expected asm output:
```asm
foo:                                    # @<!-- -->foo
        movups  xmm0, xmmword ptr [rsi]
        movaps  xmmword ptr [rdi], xmm0
        ret

Actual asm output:

foo:                                    # @<!-- -->foo
        mov     rax, qword ptr [rsi]
        mov     rcx, qword ptr [rsi + 8]
        mov     qword ptr [rdi + 8], rcx
        mov     qword ptr [rdi], rax
        ret

Replacing the two occurrences of i128 with <2 x i64> gives the expected output instead, so the issue appears to be specific to i128. Alternatively, changing the align 8 to align 16 causes LLVM to correctly output movaps instructions.

Real world background:

I noticed that filling a slice of u128 in Rust doesn't optimize as well as filling a slice of [u64; 2].

//type T = u128;
type T = [u64; 2];
pub fn foo(slice: &mut [T], value: &T) {
    slice.fill(*value)
}

[Unique-Usman]

@LingMan, I am newbie to the LLVM community, is this something which I can work on ? Thank you.

[RKSimon]

It requires someone with some familiarity with the SelectionDAG and its legalization stages.

[LingMan]

@Unique-Usman Don't ask me. This report is the only interaction I ever had with LLVM :)

[js324]

Hi @RKSimon, I’ve been trying this issue and wanted to ask for your input.

My thinking is that although the load/store of the i128 will be broken up/expanded into two i64 load/stores during legalization, the DAG combiner pass should recognize those two load/stores are adjacent and thus combine them. I initially was looking at the DagCombiner.cpp and specifically thought these load/store DAG nodes should be combining here:

https://github.com/llvm/llvm-project/blob/f87bde2962dded71599034880f57f742ea371888/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L20656-L20666

But the check was failing on the isFastLd flag (in the given example) since the unaligned memory access for 16 bytes was considered “slow” for CPUs without SSE4.2 or SSE4a:

llvm-project/llvm/lib/Target/X86/X86ISelLoweringCall.cpp

Lines 339 to 352 in ca68a14

	bool X86TargetLowering::isMemoryAccessFast(EVT VT, Align Alignment) const {
	if (isBitAligned(Alignment, VT.getSizeInBits()))
	return true;
	switch (VT.getSizeInBits()) {
	default:
	// 8-byte and under are always assumed to be fast.
	return true;
	case 128:
	return !Subtarget.isUnalignedMem16Slow();
	case 256:
	return !Subtarget.isUnalignedMem32Slow();
	// TODO: What about AVX-512 (512-bit) accesses?
	}
	}

llvm-project/llvm/lib/Target/X86/X86Subtarget.cpp

Lines 284 to 289 in ca68a14

	// All CPUs that implement SSE4.2 or SSE4A support unaligned accesses of
	// 16-bytes and under that are reasonably fast. These features were
	// introduced with Intel's Nehalem/Silvermont and AMD's Family10h
	// micro-architectures respectively.
	if (hasSSE42() || hasSSE4A())
	IsUnalignedMem16Slow = false;

But given SSE4.2 or SSE4a, the code now correctly generates:

https://llc.godbolt.org/z/nhP9T7Gv5

Do you have any tips on how to proceed/if I’m thinking about this correctly? My next thought is that I should handle this specific case of load/storing an unaligned i128 in CombineLoad and CombineStore in X86ISelLowering if at least SSE1 is given since I don’t think I should change allowsMemoryAccess() or isMemoryAccessFast() for this case?

[Gonzalosilvalde]

Hi everyone, I'd like to work on this issue. If it's currently unassigned, could someone please assign it to me? Thanks!