Missed optimization: passing vectors with different (compatible) target features prevents inlining

[tgross35]

This source:

target triple = "x86_64-unknown-linux-gnu"

define <2 x i64> @vec_entrypoint(<2 x i64> %a, <2 x i64> %b, <2 x i64> %keys) #4 {
  %r = call <2 x i64> @vec_callee1(<2 x i64> %a, <2 x i64> %b, <2 x i64> %keys)
  ret <2 x i64> %r
}

define internal <2 x i64> @vec_callee1(<2 x i64> %a, <2 x i64> %b, <2 x i64> %keys) #0 {
  %t1 = call <2 x i64> @vec_callee2(<2 x i64> %a, <2 x i64> %keys)
  ret <2 x i64> %t1
}

define internal <2 x i64> @vec_callee2(<2 x i64> %a, <2 x i64> %b) #3 {
  %r = call <2 x i64> @llvm.x86.pclmulqdq(<2 x i64> %a, <2 x i64> %b, i8 0)
  ret <2 x i64> %r
}

attributes #0 = { "target-cpu"="x86-64" }
attributes #3 = { "target-cpu"="x86-64" "target-features"="+pclmul,+sse,+sse2" }
attributes #4 = { "target-cpu"="x86-64" "target-features"="+pclmul,+sse,+sse2,+sse3" }

Emits the following suboptimal code:

vec_entrypoint:
        movaps  xmm1, xmm2
        jmp     vec_callee1

vec_callee1:
        jmp     vec_callee2

vec_callee2:
        pclmulqdq       xmm0, xmm1, 0
        ret

The middle function vec_callee doesn't have any target features enabled, and that seems to break inlining. Inlining should remain possible, however, since there can be no ABI change across the functions.

Using pointers rather than a vector return type generates the expected code:

ptr_entrypoint:                         # @ptr_entrypoint
        movdqa  xmm0, xmmword ptr [rsi]
        pclmulqdq       xmm0, xmmword ptr [rcx], 0
        movdqa  xmmword ptr [rdi], xmm0
        ret

Repro: https://llvm.godbolt.org/z/96jnrh6rP
This issue was discovered at rust-lang/rust#139029

[tgross35]

Quoting @nikic's comment rust-lang/rust#139029 (comment):

This probably runs afoul of the ABI type compatiblity check, see:

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

Lines 6524 to 6528 in b6c0ce0

	// Consider the arguments compatible if they aren't vectors or aggregates.
	// FIXME: Look at the size of vectors.
	// FIXME: Look at the element types of aggregates to see if there are vectors.
	return llvm::none_of(Types,
	[](Type *T) { return T->isVectorTy() || T->isAggregateType(); });

Basically, it's not always safe to inline a function that contains calls passing vectors if the callee enables additional target features, as this may change the way the vectors are passed.

There is no issue in this particular case, because enabling the sse4.1 feature does not change the ABI for __m128i vectors. LLVM just currently doesn't implement a precise check for this.

I think that for now it may be better to stick with passing vectors via memory, as we did before.

[tgross35]

Reduced a bit further:

target triple = "x86_64-unknown-linux-gnu"

define <2 x i64> @vec_entrypoint(<2 x i64> %a, <2 x i64> %b, <2 x i64> %keys) #3 {
  %r = call <2 x i64> @vec_callee1(<2 x i64> %a, <2 x i64> %b, <2 x i64> %keys)
  ret <2 x i64> %r
}

define internal <2 x i64> @vec_callee1(<2 x i64> %a, <2 x i64> %b, <2 x i64> %keys) #4 {
  %r = call <2 x i64> @llvm.x86.pclmulqdq(<2 x i64> %a, <2 x i64> %b, i8 0)
  ret <2 x i64> %r
}

attributes #3 = { "target-cpu"="x86-64" "target-features"="+pclmul,+sse,+sse2" }
attributes #4 = { "target-cpu"="x86-64" "target-features"="+pclmul,+sse,+sse2,+sse3" }

vec_entrypoint:                         # @vec_entrypoint
        jmp     vec_callee1                     # TAILCALL
vec_callee1:                            # @vec_callee1
        pclmulqdq       xmm0, xmm1, 0
        ret

[RalfJung]

This is somewhat related to #70563 in the sense that it's those issues around call being so context-sensitive that explain why inlining needs to be so careful here. So fundamentally this issue could be fixed either by making that check more clever (probably good to have at least as a temporary mitigation), or with more fundamental changes that avoid the need for the check entirely.