In D128806#3622477, @asb wrote:

FYI I looked before at adding a flags arg to storeRegToStackSlot, but ultimately got distracted with other things https://reviews.llvm.org/D30115

In D128806#3622380, @StephenFan wrote:

I think removing debug location is problematic. Because storeRegToStackSlot method is used in many places, for example, register spillers use it to spill a register to stack frame. If the spill instruction's debug location info were removed, its debug location would follow the previous instruction's location when emitting debug location info. It is ok to do that in -O0, but if the optimization level were -O2, instructions would be hoisted or sink which would cause the spill instruction to have wrong debug location info.

In D128806#3619189, @jrtc27 wrote:

Isn't the problem just that spill/restoreCalleeSavedRegisters need to set FrameSetup/Destroy on their stores/loads (which is also a problem with the generic implementation that we used to use before implementing -msave-restore)?

I think we may remove debug-location instead of adding FrameSetup flags for spill/restoreCalleeSavedRegisters, Because dwarf information is added for debugging convenience， At this stage, people should not care when debugging

address the comments

> Does the test actually need to check the DWARF? Can't we just check the assembly, namely that the .loc prologue_end directive is inserted in the correct place?
Need Dwarf
Through Dwarf, I am verifying that the address of the current Prologue_end is the address of the end of the fuction prologue

In D128806#3619023, @luismarques wrote:

Is removing the if (I != MBB.end()) DL = I->getDebugLoc(); actually correct? I'm wondering why do several other targets have that if it can just be removed.

In D128806#3619023, @luismarques wrote:

Is removing the if (I != MBB.end()) DL = I->getDebugLoc(); actually correct? I'm wondering why do several other targets have that if it can just be removed.

How about instead adding a MachineInstr::FrameSetup flag to the appropriate instructions? That also seems to fix the issue. Don't we want to add that flag anyway?

Does the test actually need to check the DWARF? Can't we just check the assembly, namely that the .loc prologue_end directive is inserted in the correct place?

In D125271#3545007, @pcwang-thead wrote:

In D125271#3544963, @Miss_Grape wrote:

Form your IR，

call void @llvm.masked.scatter.v4f32.v4p0(<4 x float> zeroinitializer, <4 x ptr> %1, i32 0, <4 x i1> undef)

I think this is not loop vectorization as scale-vector，set -mllvm -scalable-vectorization=off ， may also core dump？ Could you try it？

Yes, it will cause core dump even with -mllvm -scalable-vectorization=off, but will not if we remove -mllvm -riscv-v-vector-bits-max=128 -mllvm -riscv-v-vector-bits-min=128.
So, it seems that there are two problems here I guess:

1. Some loops can't be vectorized by scalable vectorization and fixed-width vectorization is took as a fallback.
2. There are some bugs in codes generated by fixed-width vectorization.

In D125747#3542925, @reames wrote:

In D125747#3542559, @craig.topper wrote:

In D125747#3541849, @Miss_Grape wrote:

In D125747#3518172, @craig.topper wrote:

Do you have any performance data?

I use the TSCV test suit, then run on the spike,

when option: -scalable-vectorization=on, the performance more better. But I'm not sure if the performance data from the spike run can be used as a standard to measure performance

Better than specifying -riscv-v-vector-bits-min to match the machine width?

@craig.topper I think this is somewhat the wrong question here. While I agree that fixed length should be our eventual default for known vector lengths, we currently don't enable any vectorization. If we can show either form of vectorization is generally profitable over the no-vectorization configuration, we should enable. We can then evaluate the other configuration against that new baseline.

@Miss_Grape I struggle to make out what that screenshot is conveying. Could you summarize please? Also, a text attachment is greatly preferred over images.

In D125271#3544933, @pcwang-thead wrote:

Here is an issue found in D125747. It seems that we will get compilation failures with -mllvm -scalable-vectorization=on using latest LLVM upstream.
Full command is: clang -O2 -march=rv64gcv -mllvm -riscv-v-vector-bits-max=128 -mllvm -riscv-v-vector-bits-min=128 -mllvm -scalable-vectorization=on .
When compiling TSVC, I got:

clang-15: llvm-project-upstream/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1172: void {anonymous}::RISCVInsertVSETVLI::emitVSETVLIs(llvm::MachineBasicBlock&): Assertion `CurInfo == Info.Exit && "InsertVSETVLI dataflow invariant violated"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: software/clang-riscv-upstream/bin/clang-15 -cc1 -triple riscv64-unknown-linux-gnu -emit-obj --mrelax-relocations -disable-free -clear-ast-before-backend -main-file-name tsc.c -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=none -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-feature +m -target-feature +a -target-feature +f -target-feature +d -target-feature +c -target-feature +v -target-feature +zve32f -target-feature +zve32x -target-feature +zve64d -target-feature +zve64f -target-feature +zve64x -target-feature +zvl128b -target-feature +zvl32b -target-feature +zvl64b -target-feature +relax -target-feature -save-restore -target-abi lp64d -msmall-data-limit 8 -mllvm -treat-scalable-fixed-error-as-warning -debugger-tuning=gdb -fcoverage-compilation-dir=workspace/issues/vectorize/TSVC -resource-dir software/clang-riscv-upstream/lib/clang/15.0.0 -isysroot software/release/sysroot/ -internal-isystem software/clang-riscv-upstream/lib/clang/15.0.0/include -internal-isystem software/release/sysroot//usr/local/include -internal-isystem software/release/lib/gcc/riscv64-unknown-linux-gnu/10.2.0/../../../../riscv64-unknown-linux-gnu/include -internal-externc-isystem software/release/sysroot//include -internal-externc-isystem software/release/sysroot//usr/include -O2 -fdebug-compilation-dir=workspace/issues/vectorize/TSVC -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fcolor-diagnostics -vectorize-loops -vectorize-slp -mllvm -riscv-v-vector-bits-max=128 -mllvm -riscv-v-vector-bits-min=128 -mllvm -scalable-vectorization=on -faddrsig -o /tmp/tsc-ef6646.o -x c tsc.c
1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'tsc.c'.
4.      Running pass 'RISCV Insert VSETVLI pass' on function '@init'
 #0 0x0000563161942c8e llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) llvm-project-upstream/llvm/lib/Support/Unix/Signals.inc:565:0
 #1 0x0000563161942d45 PrintStackTraceSignalHandler(void*) llvm-project-upstream/llvm/lib/Support/Unix/Signals.inc:632:0
 #2 0x00005631619409ec llvm::sys::RunSignalHandlers() llvm-project-upstream/llvm/lib/Support/Signals.cpp:103:0
 #3 0x000056316194260f SignalHandler(int) llvm-project-upstream/llvm/lib/Support/Unix/Signals.inc:407:0
 #4 0x00007fb119b9e980 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12980)
 #5 0x00007fb11884fe87 raise /build/glibc-uZu3wS/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #6 0x00007fb1188517f1 abort /build/glibc-uZu3wS/glibc-2.27/stdlib/abort.c:81:0
 #7 0x00007fb1188413fa __assert_fail_base /build/glibc-uZu3wS/glibc-2.27/assert/assert.c:89:0
 #8 0x00007fb118841472 (/lib/x86_64-linux-gnu/libc.so.6+0x30472)
 #9 0x000056315f2af865 (anonymous namespace)::RISCVInsertVSETVLI::emitVSETVLIs(llvm::MachineBasicBlock&) llvm-project-upstream/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1171:0
#10 0x000056315f2b0f8c (anonymous namespace)::RISCVInsertVSETVLI::runOnMachineFunction(llvm::MachineFunction&) llvm-project-upstream/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1476:0
#11 0x0000563160757cff llvm::MachineFunctionPass::runOnFunction(llvm::Function&) llvm-project-upstream/llvm/lib/CodeGen/MachineFunctionPass.cpp:73:0
#12 0x0000563160eb0894 llvm::FPPassManager::runOnFunction(llvm::Function&) llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1430:0
#13 0x0000563160eb0b3d llvm::FPPassManager::runOnModule(llvm::Module&) llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1476:0
#14 0x0000563160eb0f65 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1545:0
#15 0x0000563160eabf87 llvm::legacy::PassManagerImpl::run(llvm::Module&) llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:535:0
#16 0x0000563160eb17ef llvm::legacy::PassManager::run(llvm::Module&) llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1673:0
#17 0x0000563161e2b800 (anonymous namespace)::EmitAssemblyHelper::RunCodegenPipeline(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>&, std::unique_ptr<llvm::ToolOutputFile, std::default_delete<llvm::ToolOutputFile>>&) llvm-project-upstream/clang/lib/CodeGen/BackendUtil.cpp:1000:0
#18 0x0000563161e2ba04 (anonymous namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) llvm-project-upstream/clang/lib/CodeGen/BackendUtil.cpp:1025:0
#19 0x0000563161e2ca1c clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) llvm-project-upstream/clang/lib/CodeGen/BackendUtil.cpp:1181:0
#20 0x00005631632689b0 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) llvm-project-upstream/clang/lib/CodeGen/CodeGenAction.cpp:379:0
#21 0x0000563164ffc402 clang::ParseAST(clang::Sema&, bool, bool) llvm-project-upstream/clang/lib/Parse/ParseAST.cpp:182:0
#22 0x00005631628ea47f clang::ASTFrontendAction::ExecuteAction() llvm-project-upstream/clang/lib/Frontend/FrontendAction.cpp:1139:0
#23 0x0000563163265160 clang::CodeGenAction::ExecuteAction() llvm-project-upstream/clang/lib/CodeGen/CodeGenAction.cpp:1144:0
#24 0x00005631628e9d48 clang::FrontendAction::Execute() llvm-project-upstream/clang/lib/Frontend/FrontendAction.cpp:1036:0
#25 0x000056316281ed6b clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) llvm-project-upstream/clang/lib/Frontend/CompilerInstance.cpp:1036:0
#26 0x0000563162a89d7c clang::ExecuteCompilerInvocation(clang::CompilerInstance*) llvm-project-upstream/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:266:0
#27 0x000056315f2336b1 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) llvm-project-upstream/clang/tools/driver/cc1_main.cpp:248:0
#28 0x000056315f227195 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) llvm-project-upstream/clang/tools/driver/driver.cpp:317:0
#29 0x000056315f2278b2 main llvm-project-upstream/clang/tools/driver/driver.cpp:388:0
#30 0x00007fb118832c87 __libc_start_main /build/glibc-uZu3wS/glibc-2.27/csu/../csu/libc-start.c:344:0
#31 0x000056315f22590a _start (software/clang-riscv-upstream/bin/clang-15+0x2ce490a)

Reduced LLVM IRs via llvm-reduce:

target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
target triple = "riscv64-unknown-linux-gnu"

define i32 @init(ptr %b, i1 %0) {
entry:
  br label %vector.body8501

vector.body8501:                                  ; preds = %vector.body8501, %entry
  %vec.ind8503 = phi <4 x i64> [ %vec.ind.next8506, %vector.body8501 ], [ zeroinitializer, %entry ]
  %step.add8504 = add <4 x i64> %vec.ind8503, zeroinitializer
  %1 = getelementptr inbounds float, ptr %b, <4 x i64> %step.add8504
  call void @llvm.masked.scatter.v4f32.v4p0(<4 x float> zeroinitializer, <4 x ptr> %1, i32 0, <4 x i1> undef)
  %vec.ind.next8506 = add <4 x i64> %vec.ind8503, <i64 16, i64 16, i64 16, i64 16>
  br i1 %0, label %vector.body8513, label %vector.body8501

vector.body8513:                                  ; preds = %vector.body8513, %vector.body8501
  call void @llvm.masked.scatter.v4f32.v4p0(<4 x float> <float -1.000000e+00, float -1.000000e+00, float -1.000000e+00, float -1.000000e+00>, <4 x ptr> undef, i32 0, <4 x i1> undef)
  br label %vector.body8513
}

; Function Attrs: nocallback nofree nosync nounwind willreturn writeonly
declare void @llvm.masked.scatter.v4f32.v4p0(<4 x float>, <4 x ptr>, i32 immarg, <4 x i1>) #0

; uselistorder directives
uselistorder ptr @llvm.masked.scatter.v4f32.v4p0, { 1, 0 }

attributes #0 = { nocallback nofree nosync nounwind willreturn writeonly }

@reames can you take a look at this?

In D125747#3542925, @reames wrote:

In D125747#3542559, @craig.topper wrote:

In D125747#3541849, @Miss_Grape wrote:

In D125747#3518172, @craig.topper wrote:

Do you have any performance data?

I use the TSCV test suit, then run on the spike,

when option: -scalable-vectorization=on, the performance more better. But I'm not sure if the performance data from the spike run can be used as a standard to measure performance

Better than specifying -riscv-v-vector-bits-min to match the machine width?

@craig.topper I think this is somewhat the wrong question here. While I agree that fixed length should be our eventual default for known vector lengths, we currently don't enable any vectorization. If we can show either form of vectorization is generally profitable over the no-vectorization configuration, we should enable. We can then evaluate the other configuration against that new baseline.

@Miss_Grape I struggle to make out what that screenshot is conveying. Could you summarize please? Also, a text attachment is greatly preferred over images.

ping

In D125747#3518172, @craig.topper wrote:

Do you have any performance data?

In D123663#3450323, @nemanjai wrote:

While I certainly appreciate the work to refactor this behemoth of a function, I am about as lukewarm on how much of a readability improvement this is. If all the targets are being similarly refactored, I won't stand in the way of doing the same to PPC. So if you get the approval for the other targets, we'll go ahead with PPC as well.

ping

Add test for https://reviews.llvm.org/D125856

In D125460#3510567, @kito-cheng wrote:

hasVInstructions() check be unneeded. RISCV can only loop vectorization with hasVInstructions()

For long term, I think that's not true, P/Packed-SIMD extensions should be able to apply loop vectorization too, but I am not opposite this getting merge, just remind we might need to add back in future.

ping

In D125460#3509580, @craig.topper wrote:

LGTM, but please fix the description to use hasVInstructions instead of hasStdExtV since that's what's in the code.

ping

ping

ping

1，address comment

Fix the comment

ping

ping

If this patch Ok, need to update the following patch：
D123659 D123663 D123661 D123654 D123653

Fix comment

Add test

In D123181#3445903, @craig.topper wrote:

LGTM, but please update the commit title since you also changed becompress, bdecompress and bfp.

fix the comment

TODO: add test cases

ping

Update the test case