Might be logical error, chunk operator returns views over the input. So when you inplace modify it in chunked_hgates[1].add_(chunked_igates[1]).sigmoid_(); you are also modifying input tensor.

Thanks for the comment. I'm not sure if I correctly understand that. Here I used in_place add_ for chuncked_hgates, which comes from params.linear_hh(hidden).chunk(3, 1). The input part goes to chuncked_igates which is only used as the argument of add_. So I guess it should be safe?

in case of pre_compute_input ( btw is it better to rename it to past tense?):

 // contains vector of views over input
 const auto chunked_igates = pre_compute_input ? input.chunk(3, 1) : params.linear_ih(input).chunk(3, 1);
 
// ...

 // changing input tensor inplace!
const auto new_gate = chunked_igates[2].add(chunked_hgates[2].mul_(reset_gate)).tanh_();
 

And even if you are sure that operator() would be called only once, it is bad pattern to modify inputs

I am a little confused here. This line is "chunked_igates[2].add" which is not a in_place add_, right? And tanh_ is applied on the result of add() which is another tensor in my understanding.

My bad. LGTM.

Thanks for the detailed review.

What is wrong with the same approach in case of the GPU?

I will apply it in the next PR for GPU. Originally on GPU we do matmul instead of linear for input first, and fuse the bias part in at::_thnn_fused_lstm_cell function. So I think it will be better to make change on CPU only first then apply the GPU change in case the PR to be too large.