Skip to content

[RISCV64]: Implement CPU plugin just-in-time emitter for LogicalXor #30706

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

11happy
Copy link
Contributor

@11happy 11happy commented May 24, 2025

Overview:

Testing:

  • Tested, All tests passing :)

CC:

@11happy 11happy requested review from a team as code owners May 24, 2025 09:35
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label May 24, 2025
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label May 24, 2025
@mlukasze mlukasze requested a review from a-sidorova May 26, 2025 04:58
@a-sidorova a-sidorova self-assigned this May 26, 2025
@a-sidorova a-sidorova added this to the 2025.3 milestone May 26, 2025
@a-sidorova a-sidorova added the platform: risc-v OpenVINO on RISC-V label May 26, 2025
@@ -261,5 +261,23 @@ class jit_subtract_emitter : public jit_emitter {
void emit_isa(const std::vector<size_t>& in_vec_idxs, const std::vector<size_t>& out_vec_idxs) const;
};

class jit_xor_emitter : public jit_emitter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please align the emitter name with op name LogicalXor - it should be jit_logical_xor_emitter.

Ans also please align your changes with others in alphabetical order 😊

Comment on lines +839 to +848
switch (exec_prc_) {
case ov::element::i32:
h->vxor_vv(dst, src0, src1);
break;
case ov::element::f32:
h->vxor_vv(dst, src0, src1);
break;
default:
OV_CPU_JIT_EMITTER_THROW("Unsupported precision");
}
Copy link
Contributor

@a-sidorova a-sidorova May 26, 2025

Choose a reason for hiding this comment

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

The OpenVINO specification says: "LogicalXor op performs element-wise logical XOR operation".

But the RVV instruction performs "bitwise logical XOR operation". It means that your JIT Emitter works for "BitwiseXor" op - this is another operation with another logic.

So could you please update your implementation to support "elementwise" logical XOR operation?

The example of jit_logical_xor_emitter on ARM64.

And also for now please support only f32 case 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hii you please tell me the difference between bitwise xor & logical xor with an example it will help me with the implementation.
Thank you

Copy link
Contributor

@a-sidorova a-sidorova May 28, 2025

Choose a reason for hiding this comment

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

@11happy Hello! Let me describe simple example to distinguish bitwise xor and logical xor:

Bitwise XOR - we make XOR for each bits of values. The result of this operation new value where each bit is XOR between bits of 2 input values.
Let's imagine that we have 2 FP32 values: 1.2 and 1. Then:

[1.2]       :  00111111 10011001 10011001 10011010
[1]         :  00111111 10000000 00000000 00000000
bitwise XOR :  00000000 00011001 10011001 10011010 <- The FP32 decimal result is 2.350989e-39. This value should be in vector register

The result of bitwise xor will be number - in binary representation it is equal to xor of bits of two input values.

As for logical XOR - this is per element XOR operation. The result should be true (1) or false (0).

Copy link
Contributor Author

@11happy 11happy May 28, 2025

Choose a reason for hiding this comment

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

so would it be fine to implement in this fashion, is they are not equal then its 1 else 0 , also thank you for this descriptive example

Copy link
Contributor

@a-sidorova a-sidorova May 28, 2025

Choose a reason for hiding this comment

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

I think that probably there was mistake in my example for logical xor 🤔 And this is not just not equal or equal.

I believe that xor - is logical operation which operates with boolean values (please see specification). It means that all values which are not zero - are automatically 1 or true. Then if only operand is zero and another is true (any not-zero value), then the result is true. Otherwise - false. Like in boolean algebra :)

You can take a look at the example with umbers in pytorch documentation.

@@ -292,7 +292,8 @@ std::string EltwiseLayerCPUTest::getPrimitiveType(const utils::EltwiseTypes& elt
if ((eltwise_type == utils::EltwiseTypes::ADD) ||
(eltwise_type == utils::EltwiseTypes::SUBTRACT) ||
(eltwise_type == utils::EltwiseTypes::MULTIPLY) ||
(eltwise_type == utils::EltwiseTypes::DIVIDE)) {
(eltwise_type == utils::EltwiseTypes::DIVIDE) ||
(eltwise_type == utils::EltwiseTypes::BITWISE_XOR)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As I mentioned in the previous comment, the target operation is LogicalXor, not BitwiseXor.

logical ops are tested there. You don't need to update any checks for primitive type (getPrimitiveType) there as for eltwise/activation ops.

So please just check that the tests with --gtest_filter="*smoke*Logical*LogicalXor*" are successfully passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin ExternalPR External contributor platform: risc-v OpenVINO on RISC-V
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Good First Issue] [RISCV64]: Implement CPU plugin just-in-time emitter for LogicalXor operation
3 participants