Skip to content

[Scalarizer][Bug] Only scatter/gather in extract value for trivially vectorizable intrinsics #113624

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

Closed
farzonl opened this issue Oct 24, 2024 · 0 comments · Fixed by #113625
Closed
Assignees

Comments

@farzonl
Copy link
Member

farzonl commented Oct 24, 2024

Issue

Today we scatter/gather all extract values of struct return types. Thats out of lock step with how we split the call. The fix is to check if the call was trivially vectorizable before. handling the extract element case.

Background

This is rather assert while we are doing Scatterer::operator[] and we are creating CreateExtractElement but Type is not a Vector but a Structure. Adding uadd.with.overflow to isTriviallyScalarizable is 'fixing' this.

Thats a bug. Adding to isTriviallyScalarizable is not the right fix. Whats going on is because uadd.with.overflow isn't in isTriviallyScalarizable we aren't splitting the calls so when we call Scatterer Op0 it isn't operating on the right type. A correct fix wouldn't replace extract element unless we scalarized the call.

Test case:

define <3 x i32> @test_(<3 x i32> %a, <3 x i32> %b) {
  %r = call { <3 x i32>, <3 x i1> } @llvm.uadd.with.overflow.v3i32(<3 x i32> %b, <3 x i32> %b)
  %el = extractvalue { <3 x i32>, <3 x i1> } %r, 0
  ret <3 x i32> %el
}
./build/bin/opt -passes='function(scalarizer)' ./build/test_fail.ll -S
opt: ./llvm-project/llvm/include/llvm/Support/Casting.h:578: decltype(auto) llvm::cast(From *) [To = llvm::VectorType, From = llvm::Type]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.

Originally posted by @farzonl in #111569 (comment)

@farzonl farzonl self-assigned this Oct 24, 2024
farzonl added a commit to farzonl/llvm-project that referenced this issue Oct 24, 2024
farzonl added a commit to farzonl/llvm-project that referenced this issue Oct 24, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
Icohedron added a commit to Icohedron/llvm-project that referenced this issue Feb 11, 2025
…able

Since uadd_with_overflow is now marked isTriviallyScalarizable, another intrinsic needs to take its place for testing the bug fix introduced in llvm#113625 for issue llvm#113624.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant