Skip to content

[Scalarizer] Test *_with_overflow crash with min-bits=32 and min-bits=16 #127739

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 Feb 19, 2025 · 4 comments · Fixed by #128538
Closed

[Scalarizer] Test *_with_overflow crash with min-bits=32 and min-bits=16 #127739

farzonl opened this issue Feb 19, 2025 · 4 comments · Fixed by #128538

Comments

@farzonl
Copy link
Member

farzonl commented Feb 19, 2025

first reported here: #127520
introduced by: #126815

; RUN: opt %s -passes='function(scalarizer<load-store;min-bits=16>,dce)' -S | FileCheck %s --check-prefixes=CHECK,MIN16
; RUN: opt %s -passes='function(scalarizer<load-store;min-bits=32>,dce)' -S | FileCheck %s --check-prefixes=CHECK,MIN32

define <3 x i32> @call_v3i32(<3 x i32> %a, <3 x i32> %b) {
; CHECK-LABEL: @call_v3i32(
; CHECK-NEXT:    [[T:%.*]] = call { <3 x i32>, <3 x i1> } @llvm.uadd.with.overflow.v3i32(<3 x i32> [[A:%.*]], <3 x i32> [[B:%.*]])
; CHECK-NEXT:    [[R:%.*]] = extractvalue { <3 x i32>, <3 x i1> } [[T]], 0
; CHECK-NEXT:    ret <3 x i32> [[R]]
;
  %t = call { <3 x i32>, <3 x i1> } @llvm.uadd.with.overflow.v3i32(<3 x i32> %a, <3 x i32> %b)
  %r = extractvalue { <3 x i32>, <3 x i1> } %t, 0
  ret <3 x i32> %r
}
opt: 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.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: opt -passes=function(scalarizer<load-store;min-bits=16>,dce) -S
1.	Running pass "function(scalarizer,dce)" on module "<stdin>"
2.	Running pass "scalarizer" on function "call_v3i32"
 #0 0x0000000004028908 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (opt+0x4028908)
 #1 0x000000000402643e llvm::sys::RunSignalHandlers() (opt+0x402643e)
 #2 0x0000000004029131 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x000075c457a45330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330)
 #4 0x000075c457a9eb2c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x000075c457a9eb2c __pthread_kill_internal ./nptl/pthread_kill.c:78:10
 #6 0x000075c457a9eb2c pthread_kill ./nptl/pthread_kill.c:89:10
 #7 0x000075c457a4527e raise ./signal/../sysdeps/posix/raise.c:27:6
 #8 0x000075c457a288ff abort ./stdlib/abort.c:81:7
 #9 0x000075c457a2881b _nl_load_domain ./intl/loadmsgcat.c:1177:9
#10 0x000075c457a3b517 (/lib/x86_64-linux-gnu/libc.so.6+0x3b517)
#11 0x0000000004143bea (opt+0x4143bea)
#12 0x00000000049addf4 (anonymous namespace)::Scatterer::operator[](unsigned int) Scalarizer.cpp:0:0
#13 0x00000000049aa142 (anonymous namespace)::ScalarizerVisitor::visit(llvm::Function&) Scalarizer.cpp:0:0
#14 0x00000000049a5419 llvm::ScalarizerPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (opt+0x49a5419)

No line numbers, Looks like we have some missing debug symbols I suspect the assert is coming from ScalarizerVisitor::visitExtractValueInst

There is a use of Scatterer::operator[] as part of creating the ExtractValue IR.

for (unsigned OpIdx = 0; OpIdx < Op0.size(); ++OpIdx) {
    Value *ResElem = Builder.CreateExtractValue(
        Op0[OpIdx], Index, EVI.getName() + ".elem" + Twine(Index));
    Res.push_back(ResElem);
  }

We likely need to check VS.NumPacked before calling Scatterer::operator[]

Because that operator is using Frag * VS.NumPacked to create the exrtractElement instruction.

 CV[Frag] = Builder.CreateExtractElement(V, Frag * VS.NumPacked,
                                            V->getName() + ".i" + Twine(Frag))
@farzonl
Copy link
Member Author

farzonl commented Feb 19, 2025

@Icohedron can you look into this issue. I'm also noticing a second potential issue:

call { <3 x i32>, <3 x i1> }

The struct return elements are not the same type. one is vec3 of i32 one is and one is vec3 of i1. you need to add the *_with_overflow intrinsics to isVectorIntrinsicWithStructReturnOverloadAtField if these types are not the same.

@Icohedron
Copy link
Contributor

I'm on it

@jayfoad
Copy link
Contributor

jayfoad commented Feb 20, 2025

As mentioned in #127996, note that you can also provoke this bug by adding min-bits=64 to test/Transforms/Scalarizer/frexp.ll.

@EugeneZelenko EugeneZelenko marked this as a duplicate of #127996 Feb 20, 2025
@Icohedron
Copy link
Contributor

@Icohedron can you look into this issue. I'm also noticing a second potential issue:

call { <3 x i32>, <3 x i1> }

The struct return elements are not the same type. one is vec3 of i32 one is and one is vec3 of i1. you need to add the *_with_overflow intrinsics to isVectorIntrinsicWithStructReturnOverloadAtField if these types are not the same.

isVectorIntrinsicWithStructReturnOverloadAtField appears to only be for intrinsic name-mangling when there is more than one possible type for the the 2nd element in the struct return type (see splitCall and getOrInsertDeclaration). For the *_with_overflow intrinsics, the 2nd element of the return struct is always i1, so it does not need to be added to isVectorIntrinsicWithStructReturnOverloadAtField.

Adding the *_with_overflow intrinsic to isVectorIntrinsicWithStructReturnOverloadAtField results in a broken LLVM module.

Intrinsic name not mangled correctly for type arguments! Should be: llvm.uadd.with.overflow.i32
ptr @llvm.uadd.with.overflow.i32.i1
LLVM ERROR: Broken module found, compilation aborted!

@farzonl farzonl moved this to Needs Review in HLSL Support Feb 24, 2025
@farzonl farzonl moved this from Needs Review to Active in HLSL Support Feb 24, 2025
@Icohedron Icohedron moved this from Active to Needs Review in HLSL Support Feb 26, 2025
@github-project-automation github-project-automation bot moved this from Needs Review to Closed in HLSL Support Mar 4, 2025
@github-project-automation github-project-automation bot moved this from Needs Review to Closed in HLSL Support Mar 4, 2025
jph-13 pushed a commit to jph-13/llvm-project that referenced this issue Mar 21, 2025
…sitExtractValueInst` (llvm#128538)

Fixes llvm#127739 

The `visitExtractValueInst` is missing a check that was present in
`splitCall` / `visitCallInst`.
This check ensures that each struct element has a VectorSplit, and that
each VectorSplit contains the same number of elements packed per
fragment.

---------

Co-authored-by: Jay Foad <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
5 participants