Skip to content

[RISCV] Reject i1 vp.load/vp.store in cost model (remove LV-side workaround)#14

Closed
Copilot wants to merge 5 commits into
mainfrom
copilot/fix-issue-199896
Closed

[RISCV] Reject i1 vp.load/vp.store in cost model (remove LV-side workaround)#14
Copilot wants to merge 5 commits into
mainfrom
copilot/fix-issue-199896

Conversation

Copilot AI commented May 28, 2026

Copy link
Copy Markdown

Full-LTO RVV codegen could still carry vp.load/vp.store forms that are not representable for i1 element memory semantics on RISC-V, leading to backend failures. This change moves the handling to the RISC-V profitability/cost layer instead of guarding it in LoopVectorize transforms.

  • Problem framing

    • i1 vp.load/vp.store should be considered non-viable for RISC-V lowering.
    • Prevent this through cost invalidation, not ad-hoc mutation suppression in LoopVectorize.
  • Targeted code changes

    • RISC-V TTI cost model
      • Updated RISCVTargetTransformInfo.cpp to return InstructionCost::getInvalid() for i1 vector vp_load/vp_store in:
        • getIntrinsicInstrCost(...)
      • Kept vp.load/vp.store memory intrinsic handling routed through getMaskedMemoryOpCost(...), which already relies on masked-memory legality checks (isLegalMaskedLoadStore / isLegalElementTypeForRVV) that reject i1 element types.
    • LoopVectorize cleanup
      • Reverted the optimizeMaskToEVL i1 guard workaround in VPlanTransforms.cpp so the fix is centralized in target cost modeling.
  • Regression coverage

    • Extended llvm/test/Analysis/CostModel/RISCV/vp-intrinsics.ll with i1 VP memory cases asserting invalid cost.
    • Removed the temporary LoopVectorize regression test added for the prior workaround approach.

This fixes a crash where the vector legalizer tried to unroll a
scalable vector vp_load of a mask type (e.g. nxv16i1), which called
getVectorNumElements() on a scalable vector.

The loop vectorizer can produce vp.load/vp.store of mask types when
loading _Bool arrays that are subsequently used as booleans. The RISC-V
backend was missing Custom lowering for VP_LOAD/VP_STORE on BoolVecVTs,
causing the vector legalizer to fall through to Expand/Unroll which
doesn't support scalable vectors.

The fix adds VP_LOAD/VP_STORE as Custom for all boolean vector types
and handles them in lowerMaskedLoad/lowerMaskedStore by emitting
riscv_vlm/riscv_vsm intrinsics respectively.

Fixes llvm#199896

Co-authored-by: lukel97 <2488460+lukel97@users.noreply.github.com>
Copilot AI changed the title [RISCV] Add VP_LOAD/VP_STORE support for mask (i1) vector types [RISCV] Add VP_LOAD/VP_STORE lowering for mask vector types May 28, 2026
Copilot AI requested a review from lukel97 May 28, 2026 09:43
RVV's vlm.v/vsm.m instructions don't support a mask operand, so
vp.load/vp.store of i1 types cannot be correctly lowered. The mask
would be silently dropped during lowering. Add checks in
optimizeMaskToEVL to skip the conversion of masked loads/stores to
EVL form when the element type is i1.

Co-authored-by: lukel97 <2488460+lukel97@users.noreply.github.com>
The VPlanTransforms.cpp fix (preventing i1 types in optimizeMaskToEVL) is
sufficient to fix the crash. Since optimizeEVLMasks runs before the plan cost
is computed, i1 loads/stores will remain as masked loads/stores rather than
being converted to vp.load/vp.store, avoiding the RISC-V backend crash.

Remove the vp.load/vp.store RISCV lowering changes and their CodeGen tests,
and add a LoopVectorize test for issue llvm#199896.

Co-authored-by: lukel97 <2488460+lukel97@users.noreply.github.com>
Copilot AI changed the title [VPlan] Skip EVL conversion for i1 load/store in optimizeMaskToEVL [LoopVectorize] Prevent optimizeMaskToEVL from creating vp.load/vp.store of i1 types May 28, 2026
Revert the LoopVectorize-side guard in optimizeMaskToEVL and model i1
vp.load/vp.store as invalid in RISC-V TTI instead.

RISC-V has no native masked memory op with vp.load/vp.store semantics
for i1 element types (vlm/vsm lack a mask operand), so plans that rely on
these operations should be rejected by profitability/cost checks.

This updates both mem intrinsic and intrinsic cost hooks for vp_load/
vp_store i1 vectors and adds a cost-model regression test.

Also remove the temporary LoopVectorize regression test added for the
previous approach.

Co-authored-by: lukel97 <2488460+lukel97@users.noreply.github.com>
Copilot AI changed the title [LoopVectorize] Prevent optimizeMaskToEVL from creating vp.load/vp.store of i1 types [RISCV] Reject i1 vp.load/vp.store in cost model (remove LV-side workaround) May 28, 2026
Comment on lines 1080 to 1083
[[fallthrough]];
case Intrinsic::masked_load:
case Intrinsic::masked_store:
return getMaskedMemoryOpCost(MICA, CostKind);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot doesn't getMaskedMemoryOpCost already check for isLegalMaskedLoadStore, which in turn already checks for TLI->isLegalElementTypeForRVV ? Which should disallow i1.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — getMaskedMemoryOpCost already funnels illegal i1 through BaseT::getMemIntrinsicInstrCost, which makes vp.load/vp.store invalid. I removed the redundant mem-intrinsic-side i1 guard in 7c1ef91.

I kept the getIntrinsicInstrCost i1 check because getInstructionCost for intrinsic calls goes through that path directly.

Co-authored-by: lukel97 <2488460+lukel97@users.noreply.github.com>
Copilot AI requested a review from lukel97 May 28, 2026 13:44
@lukel97 lukel97 closed this Jun 1, 2026
lukel97 pushed a commit that referenced this pull request Jun 9, 2026
I recently noticed LLDB crash during execution of `script
print(lldb.SBDebugger().GetBroadcaster().GetName())` command:
```
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/sergei/llvm-project/build/bin/lldb-dap
 #0 0x000062735c3403d2 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/sergei/llvm-project/build/bin/lldb-dap+0x7c3d2)
 #1 0x000062735c33d7ec llvm::sys::RunSignalHandlers() (/home/sergei/llvm-project/build/bin/lldb-dap+0x797ec)
 #2 0x000062735c33d94c SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007eaa6aa45330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330)
 #4 0x00007eaa6bb0c092 lldb::SBBroadcaster::GetName() const (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0x90c092)
 #5 0x00007eaa6bcb9a5d _wrap_SBBroadcaster_GetName LLDBWrapPython.cpp:0:0
 #6 0x00007eaa6a1df5f5 (/lib/x86_64-linux-gnu/libpython3.12.so.1.0+0x1df5f5)
 #7 0x00007eaa6a182b2c PyObject_Vectorcall (/lib/x86_64-linux-gnu/libpython3.12.so.1.0+0x182b2c)
 #8 0x00007eaa6a11d5ee _PyEval_EvalFrameDefault (/lib/x86_64-linux-gnu/libpython3.12.so.1.0+0x11d5ee)
 #9 0x00007eaa6a2a091f PyEval_EvalCode (/lib/x86_64-linux-gnu/libpython3.12.so.1.0+0x2a091f)
#10 0x00007eaa6a29c8b0 (/lib/x86_64-linux-gnu/libpython3.12.so.1.0+0x29c8b0)
#11 0x00007eaa6a11fbd3 _PyEval_EvalFrameDefault (/lib/x86_64-linux-gnu/libpython3.12.so.1.0+0x11fbd3)
#12 0x00007eaa6c4891b7 lldb_private::ScriptInterpreterPythonImpl::ExecuteOneLine(llvm::StringRef, lldb_private::CommandReturnObject*, lldb_private::ExecuteScriptOptions const&) (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0x12891b7)
#13 0x00007eaa70326ff5 CommandObjectScriptingRun::DoExecute(llvm::StringRef, lldb_private::CommandReturnObject&) (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0x5126ff5)
#14 0x00007eaa6bee3739 lldb_private::CommandObjectRaw::Execute(char const*, lldb_private::CommandReturnObject&) (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0xce3739)
#15 0x00007eaa6bede09a lldb_private::CommandInterpreter::HandleCommand(char const*, lldb_private::LazyBool, lldb_private::CommandReturnObject&, bool) (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0xcde09a)
#16 0x00007eaa6bb0f0f8 lldb::SBCommandInterpreter::HandleCommand(char const*, lldb::SBExecutionContext&, lldb::SBCommandReturnObject&, bool) (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0x90f0f8)
llvm#17 0x00007eaa6bb0f265 lldb::SBCommandInterpreter::HandleCommand(char const*, lldb::SBCommandReturnObject&, bool) (/home/sergei/llvm-project/build/bin/../lib/liblldb.so.23.0git+0x90f265)
llvm#18 0x000062735c3707f3 lldb_dap::RunLLDBCommands[abi:cxx11](lldb::SBDebugger&, lldb::SBMutex, llvm::StringRef, llvm::ArrayRef<lldb_dap::protocol::String> const&, bool&, bool, bool) (/home/sergei/llvm-project/build/bin/lldb-dap+0xac7f3)
llvm#19 0x000062735c3a8019 lldb_dap::EvaluateRequestHandler::Run(lldb_dap::protocol::EvaluateArguments const&) const (/home/sergei/llvm-project/build/bin/lldb-dap+0xe4019)
llvm#20 0x000062735c3aba78 lldb_dap::RequestHandler<lldb_dap::protocol::EvaluateArguments, llvm::Expected<lldb_dap::protocol::EvaluateResponseBody>>::operator()(lldb_dap::protocol::Request const&) const (/home/sergei/llvm-project/build/bin/lldb-dap+0xe7a78)
llvm#21 0x000062735c3ce1bf lldb_dap::BaseRequestHandler::Run(lldb_dap::protocol::Request const&) (/home/sergei/llvm-project/build/bin/lldb-dap+0x10a1bf)
llvm#22 0x000062735c3577e7 lldb_dap::DAP::HandleObject(std::variant<lldb_dap::protocol::Request, lldb_dap::protocol::Response, lldb_dap::protocol::Event> const&) (/home/sergei/llvm-project/build/bin/lldb-dap+0x937e7)
llvm#23 0x000062735c358705 lldb_dap::DAP::Loop() (/home/sergei/llvm-project/build/bin/lldb-dap+0x94705)
llvm#24 0x000062735c2ed0c7 main (/home/sergei/llvm-project/build/bin/lldb-dap+0x290c7)
llvm#25 0x00007eaa6aa2a1ca __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
```
As far as I understand default constuctors should be covered by fuzzing
tests, so I don't know how to write test for that patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants