Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inlining can cause vector ABI mismatch #52660

Closed
nikic opened this issue Dec 13, 2021 · 3 comments
Closed

Inlining can cause vector ABI mismatch #52660

nikic opened this issue Dec 13, 2021 · 3 comments
Assignees
Labels
ABI Application Binary Interface backend:X86

Comments

@nikic
Copy link
Contributor

nikic commented Dec 13, 2021

Consider the following input IR:

target triple = "x86_64-unknown-linux-gnu"
 
define void @test1() "target-features"="+avx" {
  call void @test2() 
  ret void
} 
 
define internal void @test2() {
  call i64 @test3(<4 x i64> <i64 0, i64 1, i64 2, i64 3>)
  ret void
}
 
define i64 @test3(<4 x i64> %arg) noinline {
  %v = extractelement <4 x i64> %arg, i64 2
  ret i64 %v
}

Running this through opt -inline produces:


define void @test1() #0 {
  %1 = call i64 @test3(<4 x i64> <i64 0, i64 1, i64 2, i64 3>)
  ret void
}

define i64 @test3(<4 x i64> %arg) #1 {
  %v = extractelement <4 x i64> %arg, i64 2
  ret i64 %v
}

attributes #0 = { "target-features"="+avx" }
attributes #1 = { noinline }

This inlining is allowed, because test1 has a superset of target features of test2. However, what is now going to happen is that the backend will lower the test3() call using ymm registers (because the function has avx target feature), while the function will expect the arguments in xmm registers (because it does not have avx target feature).

Godbolt: https://llvm.godbolt.org/z/M95svT6qY

I previously started a discussion about this on the mailing list, but did not get much response: https://groups.google.com/g/llvm-dev/c/g_6THpxasjA

(Downstream reports of this miscompile are rust-lang/rust#79865 and rust-lang/rust#91839.)

@nikic nikic added ipo Interprocedural optimizations ABI Application Binary Interface labels Dec 13, 2021
@nikic nikic self-assigned this Dec 15, 2021
@nikic
Copy link
Contributor Author

nikic commented Dec 15, 2021

The idea to determine call ABI based on callee target features does not work out because we don't always know the callee. While the callee function type and the calling convention are part of the call, the target features are part of the callee function, which may be unknown (indirect call).

So I think the only option here is to prevent inlining in the first place.

@RalfJung
Copy link
Contributor

RalfJung commented Aug 29, 2024

Fix this by scanning over all calls in the function and checking
whether ABI incompatibility is possible. Calls that only pass scalar
types are excluded, as I believe those always use the same ABI
independent of target features.

FWIW this is not quite true -- the x87 and softfloat target features affect the ABI of float and double values.

I also wonder, what about other targets than X86? Won't they need the same treatment?

@EugeneZelenko EugeneZelenko added backend:X86 and removed ipo Interprocedural optimizations labels Aug 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 29, 2024

@llvm/issue-subscribers-backend-x86

Author: Nikita Popov (nikic)

Consider the following input IR: ``` target triple = "x86_64-unknown-linux-gnu"

define void @test1() "target-features"="+avx" {
call void @test2()
ret void
}

define internal void @test2() {
call i64 @test3(<4 x i64> <i64 0, i64 1, i64 2, i64 3>)
ret void
}

define i64 @test3(<4 x i64> %arg) noinline {
%v = extractelement <4 x i64> %arg, i64 2
ret i64 %v
}

Running this through `opt -inline` produces:
```target triple = "x86_64-unknown-linux-gnu"

define void @<!-- -->test1() #<!-- -->0 {
  %1 = call i64 @<!-- -->test3(&lt;4 x i64&gt; &lt;i64 0, i64 1, i64 2, i64 3&gt;)
  ret void
}

define i64 @<!-- -->test3(&lt;4 x i64&gt; %arg) #<!-- -->1 {
  %v = extractelement &lt;4 x i64&gt; %arg, i64 2
  ret i64 %v
}

attributes #<!-- -->0 = { "target-features"="+avx" }
attributes #<!-- -->1 = { noinline }

This inlining is allowed, because test1 has a superset of target features of test2. However, what is now going to happen is that the backend will lower the test3() call using ymm registers (because the function has avx target feature), while the function will expect the arguments in xmm registers (because it does not have avx target feature).

Godbolt: https://llvm.godbolt.org/z/M95svT6qY

I previously started a discussion about this on the mailing list, but did not get much response: https://groups.google.com/g/llvm-dev/c/g_6THpxasjA

(Downstream reports of this miscompile are rust-lang/rust#79865 and rust-lang/rust#91839.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Application Binary Interface backend:X86
Projects
None yet
Development

No branches or pull requests

4 participants