Fix the issue #24114 #24278

Aryan8912 · 2024-04-28T03:07:49Z

close: #24114

Details:

item1
...

Tickets:

[Good First Issue] [ARM]: Implement CPU plugin just-in-time emitter for SoftSign operation #24114

eshoguli · 2024-04-29T13:27:51Z

You implemented JIT emitter: good start, thanks!

I would like to suggest next implementation steps.

Update tests to be sure that your implementation is used. Tests should fail: acl_f32 primitive is used.
Update JIT kernel. Tests should fail: expected and reference tensors comparation fails.
Finish JIT emitter. Tests should pass.

Let me know if you still have any questions.

eshoguli · 2024-04-29T13:28:45Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.hpp

@@ -309,6 +309,27 @@ class jit_select_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_softsign_emitter : public jit_emitter {


please rename: jit_softsign_emitter => jit_soft_sign_emitter

eshoguli · 2024-04-29T13:32:55Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+    TReg aux = TReg(aux_vec_idxs[0]);
+    h->movi(aux.s, 1);
+    h->fdiv(dst.s, src.s, aux.s);
+    h->fadd(dst.s, dst.s, src.s);
+    h->fdiv(dst.s, dst.s, aux.s);
+    h->fsub(dst.s, aux.s, dst.s);
+    h->fmul(dst.s, dst.s, src.s);


can you align implementation with OpenVINO SoftSign operation from here: https://docs.openvino.ai/2022.3/openvino_docs_ops_activation_SoftSign_9.html

you don't need auxiliary register here

…nvinotoolkit#24109

…into aryan_softsign

eshoguli · 2024-05-16T13:05:13Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+
+// SoftPlus
+
+jit_softplus_emitter::jit_softplus_emitter(dnnl::impl::cpu::aarch64::jit_generator *host,


Move here: #24109, please

eshoguli · 2024-05-16T13:05:36Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.hpp

+class jit_soft_sign_emitter : public jit_emitter {
+    jit_soft_sign_emitter(dnnl::impl::cpu::aarch64::jit_generator *host,
+                           dnnl::impl::cpu::aarch64::cpu_isa_t host_isa,
+                           const ov::element::Type exec_prc = ov::element::f32);
+                           jit_soft_sign_emitter(dnnl::impl::cpu::aarch64::jit_generator *host,
+                                                   dnnl::impl::cpu::aarch64::cpu_isa_t host_isa,
+                                                   const std::shared_ptr<ov::Node>& node);
+                                                   size_t get_inputs_count() const override;
+                                                   size_t get_aux_vecs_count() const override;
+                                                   static std::set<std::vector<element::Type>> get_supported_precisions(const std::shared_ptr<ov::Node>& node = nullptr);
+                                                   private:
+                                                   void emit_impl(const std::vector<size_t> &in_vec_idxs, const std::vector<size_t> &out_vec_idxs) const override;
+                                                   template <dnnl::impl::cpu::aarch64::cpu_isa_t isa>
+                                                   void emit_isa(const std::vector<size_t> &in_vec_idxs, const std::vector<size_t> &out_vec_idxs) const;
+
+};


fix alignment, please

Aryan8912 · 2024-05-17T15:43:26Z

@eshoguli I think this is working fine but the problem is a test case does not respond?

eshoguli · 2024-05-20T16:05:25Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+    h->movi(aux.s, 1);
+    h->fdiv(dst.s, src.s, 1); 
+    h->fadd(dst.s, dst.s, src.s);
+    h->fdiv(dst.s, dst.s, 1); 
+    h->fsub(dst.s, 1, dst.s);  
+    h->fmul(dst.s, dst.s, src.s);


Please fix, SoftSign implementation: https://docs.openvino.ai/2022.3/openvino_docs_ops_activation_SoftSign_9.html

y = x x = abs(x) x = x + 1 y = y / x

eshoguli · 2024-05-20T16:10:12Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

@@ -896,6 +896,50 @@ void jit_select_emitter::emit_isa(const std::vector<size_t> &in_vec_idxs, const
    h->mov(dst.b16, aux.b16);
 }

+// SoftSign
+
+jit_softsign_emitter::jit_softsign_emitter(dnnl::impl::cpu::aarch64::jit_generator *host,


jit_softsign_emitter => jit_soft_sign_emitter

eshoguli

Comments about implementation: you can find SoftSign explanation here: https://docs.openvino.ai/2024/documentation/openvino-ir-format/operation-sets/operation-specs/activation/softsign-9.html.

It's simple:

tmp = x
x = abs(x)
x = x + 1
y = tmp / x

Note, please, source and destination register can be the same.

eshoguli · 2024-05-24T09:24:14Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+    TReg src = TReg(in_vec_idxs[0]);
+    TReg dst = TReg(out_vec_idxs[0]);
+    TReg aux = TReg(aux_vec_idxs[0]);
+    h->fabs(src.s, src.s);      


You are losing the original input value here, which has to used for the last step: x / (...). You should use auxiliary register to store the abs(x) result value, not destination register.

eshoguli · 2024-05-24T09:28:00Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+    h->fcvtns(dst.d, src.s);   
+    h->fld1(dst.d, dst.d);    


you don't need it here

operate with word (float32) values

eshoguli · 2024-05-24T09:28:49Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+    h->fcvtns(dst.d, src.s);   
+    h->fld1(dst.d, dst.d);    
+    h->fadd(dst.d, dst.d, src.d); 
+    h->flog(dst.d, dst.d);     


don't need it here

eshoguli · 2024-05-24T09:29:02Z

src/plugins/intel_cpu/src/emitters/plugin/aarch64/jit_eltwise_emitters.cpp

+    h->fadd(dst.d, dst.d, src.d); 
+    h->flog(dst.d, dst.d);     
+    h->fdiv(dst.d, dst.d, v2.d); 
+    h->fmuls(src.s, dst.s, src.s); 


don't need it here

@eshoguli please provide more detail

mlukasze · 2024-06-18T09:00:05Z

hey @Aryan8912 will you have a time to finish this PR?

github-actions · 2024-07-03T00:19:40Z

This PR will be closed in a week because of 2 weeks of no activity.

Fix the issue openvinotoolkit#24114

f8e620b

Aryan8912 requested review from a team as code owners April 28, 2024 03:07

github-actions bot added the category: CPU OpenVINO CPU plugin label Apr 28, 2024

sys-openvino-ci added the ExternalPR External contributor label Apr 28, 2024

ilya-lavrenov added the platform: arm OpenVINO on ARM / ARM64 label Apr 28, 2024

ilya-lavrenov assigned eshoguli Apr 28, 2024

eshoguli reviewed Apr 29, 2024

View reviewed changes

Aryan8912 added 3 commits May 5, 2024 17:14

Update jit_eltwise_emitters.cpp

5a6f252

Update jit_eltwise_emitters.hpp

de40e6f

Update jit_eltwise_emitters.hpp

8696e6c

dmitry-gorokhov added this to the 2024.2 milestone May 6, 2024

Aryan8912 and others added 6 commits May 6, 2024 14:49

Implement CPU plugin just-in-time emitter for SoftPlus operation ope…

c2a967d

…nvinotoolkit#24109

Update SoftSign Function

d444c68

Merge branch 'aryan_softsign' of https://github.com/Aryan8912/openvino …

6e9c8c6

…into aryan_softsign

reupdate function

0f26090

Merge branch 'master' into aryan_softsign

7d3e50a

Merge branch 'master' into aryan_softsign

d1b1aa2

eshoguli reviewed May 16, 2024

View reviewed changes

Aryan8912 added 4 commits May 16, 2024 22:56

Update jit_eltwise_emitters.hpp

479377d

Update jit_eltwise_emitters.cpp

304ff2b

Merge branch 'master' into aryan_softsign

359a3b3

Update jit_eltwise_emitters.hpp

2106320

eshoguli reviewed May 20, 2024

View reviewed changes

Aryan8912 added 2 commits May 21, 2024 00:46

Update jit_eltwise_emitters.cpp

9e35deb

Update jit_eltwise_emitters.cpp

819d1b6

dmitry-gorokhov removed this from the 2024.2 milestone May 23, 2024

dmitry-gorokhov added this to the 2024.3 milestone May 23, 2024

eshoguli reviewed May 24, 2024

View reviewed changes

github-actions bot added the Stale label Jul 3, 2024

mg-intel removed the Stale label Jul 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the issue #24114 #24278

Fix the issue #24114 #24278

Aryan8912 commented Apr 28, 2024 •

edited by eshoguli

Loading

eshoguli commented Apr 29, 2024

eshoguli Apr 29, 2024

eshoguli Apr 29, 2024

eshoguli May 16, 2024

eshoguli May 16, 2024

Aryan8912 commented May 17, 2024

eshoguli May 20, 2024

eshoguli May 20, 2024

eshoguli left a comment

eshoguli May 24, 2024

eshoguli May 24, 2024 •

edited

Loading

eshoguli May 24, 2024

eshoguli May 24, 2024

Aryan8912 Jun 6, 2024

mlukasze commented Jun 18, 2024

github-actions bot commented Jul 3, 2024


		// SoftPlus

		jit_softplus_emitter::jit_softplus_emitter(dnnl::impl::cpu::aarch64::jit_generator *host,

Fix the issue #24114 #24278

Are you sure you want to change the base?

Fix the issue #24114 #24278

Conversation

Aryan8912 commented Apr 28, 2024 • edited by eshoguli Loading

Details:

Tickets:

eshoguli commented Apr 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aryan8912 commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eshoguli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eshoguli May 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlukasze commented Jun 18, 2024

github-actions bot commented Jul 3, 2024

Aryan8912 commented Apr 28, 2024 •

edited by eshoguli

Loading

eshoguli May 24, 2024 •

edited

Loading