Skip to content
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

[RISC-V] Do not add vector intrinsics with lanes=1 #7192

Closed
wants to merge 1 commit into from

Conversation

dkurt
Copy link
Contributor

@dkurt dkurt commented Nov 30, 2022

Currently, LLVM throws the following error when vector_bits=64:

Intrinsic has incorrect return type!
ptr @llvm.riscv.vaadd.i64.i64.i64
Intrinsic has incorrect return type!
ptr @llvm.riscv.vaaddu.i64.i64.i64

corresponding intrinsic code:

; Function Attrs: alwaysinline nounwind
define internal i64 @"halving_add_wrapper$3"(i64 %0, i64 %1) #9 {
entry:
  call void asm sideeffect "csrw vxrm,${0:z}", "rJ,~{memory}"(i64 2)
  %2 = call i64 @llvm.riscv.vaadd.i64.i64.i64(i64 undef, i64 %0, i64 %1, i64 1)
  ret i64 %2
}
Printer
--- a/src/CodeGen_RISCV.cpp
+++ b/src/CodeGen_RISCV.cpp
@@ -381,6 +381,11 @@ llvm::Function *CodeGen_RISCV::define_riscv_intrinsic_wrapper(const RISCVIntrins
     function_does_not_access_memory(wrapper);
     wrapper->addFnAttr(llvm::Attribute::NoUnwind);
 
+    std::string str;
+    llvm::raw_string_ostream output(str);
+    wrapper->print(output);
+    std::cout << str << std::endl;
+
     llvm::verifyFunction(*wrapper);
     return wrapper;
 }
Reproducer
#include <Halide.h>

using namespace Halide;

const int width = 640;
const int height = 480;

int main(int argc, char** argv) {
    Func f("f");
    Buffer<uint8_t> input(width, height);

    Var x("x"), y("y");
    f(x, y) = input(x, y);

    f.vectorize(x, 8);

    Target target = get_host_target();
    target.vector_bits = 8 * sizeof(uint8_t) * 8;

    std::vector<Target::Feature> features;
    features.push_back(Target::RVV);
    target.set_features(features);

    std::cout << target << std::endl;

    f.print_loop_nest();

    Buffer<uint8_t> output(width, height);
    f.realize(output, target);
}

Solution is to enable vaadd and others with <vscale x 1 x i64> or just skip as proposed in this PR.

@steven-johnson
Copy link
Contributor

Is this still active?

@dkurt
Copy link
Contributor Author

dkurt commented Dec 7, 2022

Is this still active?

Yes, it's still actual problem.

@zvookin
Copy link
Member

zvookin commented Dec 7, 2022

I'm pretty sure it is necessary to make this work to get fractional LMUL support. I'll take a look at it soon.

@steven-johnson
Copy link
Contributor

This has been languishing a long time; I suspect @zvookin won't be able to look at it anytime soon; is there a process for getting this landed?

@zvookin
Copy link
Member

zvookin commented Feb 7, 2023

I'm hoping to get a PR up today that does RISC V intrinsics without using the find intrinsics mechanism. I don't think it is possible to do this well by registering all the widths.

@steven-johnson
Copy link
Contributor

Monday Morning Review Ping

@steven-johnson
Copy link
Contributor

Friday Morning Review Ping

@zvookin
Copy link
Member

zvookin commented Jul 1, 2023

This is obsolete now.

@zvookin zvookin closed this Jul 1, 2023
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.

3 participants