I would like to ask a question, is this patch to reduce code size?

Thanks all.

In D128250#3617617, @Emmmer wrote:

In D128250#3617409, @liaolucy wrote:

I try to build lldb with the patch, but fail:

The environment：

cat /proc/cpuinfo 
processor       : 0
hart            : 2
isa             : rv64imafdc
mmu             : sv39
uarch           : sifive,u74-mc

llvm commit: d1b098fc

Command: cmake -DLLVM_TARGETS_TO_BUILD="RISCV" -DLLVM_ENABLE_PROJECTS="lldb" -DCMAKE_BUILD_TYPE="Release" -G Ninja ../llvm

Error:

CMake Error at /home/liaochunyu/llvm-project/lldb/cmake/modules/LLDBConfig.cmake:289 (message):
  Expected directory for clang-resource headers not found:
Call Stack (most recent call first):
  /home/liaochunyu/llvm-project/lldb/CMakeLists.txt:28 (include)


-- Configuring incomplete, errors occurred!

The most convenient way to handle clang dependencies is adding clang to LLVM_ENABLE_PROJECTS

-DLLVM_ENABLE_PROJECTS="lldb;clang"

I try to build lldb with the patch, but fail:

LGTM, thanks

The patch : fix crash for rv32 vget intrinsics

In D127447#3591261, @craig.topper wrote:

In D127447#3590942, @liaolucy wrote:

Don't we need to qualify Opcode?

D126372, I tried to implement a preliminary support, referring to other architectures limiting some opcode

Isn't the opcode checked on line 356?

if (ISD != ISD::ADD && ISD != ISD::OR && ISD != ISD::XOR && ISD != ISD::AND &&
      ISD != ISD::FADD)

address craig.topper's comments and thanks

Don't we need to qualify Opcode?

After a series of @reames's patches, no crash, thanks.

warning:

llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp:339:6: warning: ‘bool {anonymous}::AMDGPUPerfHint::isConstantAddr(const llvm::Value*) const’ defined but not used [-Wunused-function]
  339 | bool AMDGPUPerfHint::isConstantAddr(const Value *V) const {
      |      ^~~~~~~~~~~~~~

warning:

llvm-project/llvm/lib/IR/Core.cpp:90:18: warning: cast between incompatible function types from ‘LLVMDiagnosticHandler’ {aka ‘void (*)(LLVMOpaqueDiagnosticInfo*, void*)’} to ‘llvm::DiagnosticHandler::DiagnosticHandlerTy’ {aka ‘void (*)(const llvm::DiagnosticInfo&, void*)’} [-Wcast-function-type]
   90 |           Handler),
      |                  ^

The asm of testcase before this patch:

Address @craig.topper's comment.

In D123799#3457920, @craig.topper wrote:

In D123799#3457909, @liaolucy wrote:

I would like to ask a question:

What are the benefits of illegal costs? Is there a problem that illegal costs are trying to solve?

It prevents the vectorizer from creating llvm.sin, etc. with scalable vector types since they will crash in the backend.

1. Add a illegal checks. Invalid types will crash.

if(!isTypeLegal(VTy))
  return BaseT::getArithmeticReductionCost(Opcode, VTy, FMF, CostKind);

2. Remove deadcode.

Maybe we can not return invalid costs. Just reference D123799.

In D126273#3534429, @craig.topper wrote:

In D126273#3533580, @liaolucy wrote:

When I test tsvc.

The IR is:

@llvm.fmuladd.nxv2f32(<vscale x 2 x float>.....

Not

@llvm.riscv.vfmul.nxv2f32.nxv2f32(<vscale x 2 x float> ......
@llvm.riscv.vfadd.nxv2f32.nxv2f32(<vscale x 2 x float> ......

So I am not sure, we need merge vp.fmul and vp.fadd to vp.fma. maybe vp.fma is enough?

fmuladd is use with -ffp-contract=on(the default). fmul+fadd is used with -ffast-math or -ffp-contract=fast or -Ofast.

In D126273#3534004, @fakepaper56 wrote:

In D126273#3533580, @liaolucy wrote:

When I test tsvc.

The IR is:

@llvm.fmuladd.nxv2f32(<vscale x 2 x float>.....

Not

@llvm.riscv.vfmul.nxv2f32.nxv2f32(<vscale x 2 x float> ......
@llvm.riscv.vfadd.nxv2f32.nxv2f32(<vscale x 2 x float> ......

So I am not sure, we need merge vp.fmul and vp.fadd to vp.fma. maybe vp.fma is enough?

I am not sure what is tscv. Could you tell its full name?
But I think the case may happen when loop vectorizer generates vector prediction intrinsics.

tsvc:
https://github.com/llvm/llvm-test-suite/blob/main/MultiSource/Benchmarks/TSVC/tsc.inc

When I test tsvc.

thanks craig.topper, testcases add +experimental-zvfh.
change LT.first + 1 to LT.first * 2

Address reames's comments. rebase

These new testcases are placed to the end of the file, and rebase

Why do D125866 need loop vectorization? Is the performance improved?

LGTM, thanks

The testcase already has arch attribute, so assemble disassemble are work fine as past, the difference is disassembler using RISCVISAInfo to parse arch string from ELF attribute.

Does this test case need to be updated? llvm/test/MC/RISCV/attribute-with-insts.s

Thanks craig.topper for the quick fix. rebase

I would like to ask a question:

Thanks Kito. Rebase

This function assumes fixed-width vectors and cannot be used for scalable vectors. I'm not sure what returning 'Invalid' here would really fix, other than the compiler not crashing for a use-case that should not have occurred in the first place, because an overloaded cost function should have been implemented. Whether the compiler falls into the assert from cast<FixedVectorType>, or whether it returns an Invalid cost, in either case you'll need to implement a memory-op-cost function for your target. And because you'll need to implement a cost-function anyway, returning Invalid doesn't really make a difference, because then this code will never be hit. Otherwise, you would have been able to write a test-case for it.

In D121677#3386240, @frasercrmck wrote:

I actually hit this same issue the day you posted this, which is fun. But I fear this is quite a lot of work to get watertight. I've left some comments showing where we'd still crash. I've also seen us crash on getIntrinsicInstrCost when given, e.g., llvm.cttz.nxv1i8.

Can adding a cost model for llvm.cttz.nxv1i8 solve this problem? I would like to try.

https://reviews.llvm.org/D119529 maybe this patch , try to fix the same issue

I try to support scalable vectors in getGatherScatterOpCost, but I don't know how to support scalable vectors.

The scalar crypto v1.0 builtins/intrinsics is still under discussion. Maybe we should wait for the final results？

clang Zbkc patch: https://reviews.llvm.org/D112774， If there are any mistakes, you can help to point them out

Do you add intrinsic?

LGTM

LGTM, thanks

LGTM, thanks

Is there a missing test for fcvt.h.d in the test case?

In D92715#2495897, @HsiangKai wrote:

@liaolucy RISC-V vector types are sizeless types. Sizeless is kind of characteristic for builtin types. If we use attribute to declare RISC-V vector types, the frontend does not know anything about it. I still think to define RISC-V vector types as builtin types is a better way.