LDE cleanup, omit redundant argument and fix larger domain sizes.

[dot-asm]

I also have question about LDE_aux. I don't see that aux_data is used for anything. The DtoH transfers only ext_domain_data and then dev_ptr_t is freed hence simply omitting aux_data. What am I missing?

[dot-asm]

And an additional thought, also in the LDE_aux() context. As well as in LDE(). The first commit removes ext_pow argument, but if anything, it looks like it's the LDE_aux() and LDE() methods that might have a use for it... I mean LDE_spread_distribute_powers kernel is the one where ext_pow is [allededly] meaningful, but no public method lets application specify it as an option...

[sandsentinel]

I also have question about LDE_aux. I don't see that aux_data is used for anything. The DtoH transfers only ext_domain_data and then dev_ptr_t is freed hence simply omitting aux_data. What am I missing?

It's a bit of a mess. LDE_aux was meant to be something where one can also get the intermediate data in natural-order, but current version doesn't return the intermediate data.

[sandsentinel]

it looks like it's the LDE_aux() and LDE() methods that might have a use for it...

ext_pow is needed, but I realise now that the way I went about it to make it available as an argument is insufficient. Can you keep ext_pow there for now, and I will prepare a better way to pass around ext_pow as well as fix LDE_aux?

[dot-asm]

LDE_aux was meant to be something

It doesn't matter what is was meant to be, once exposed to the public all that matter what it is :-) Anyway, see additional commit.

[sandsentinel]

Anyway, see additional commit.

Looks good. With this change I think we can replace LDE with LDE_aux.

[dot-asm]

ext_pow is needed,

No, it's not. In all cases the existing parameters are sufficient to describe any particular operation. As commit message says "In the LDE_powers() case the operation is fully described by |lg_blowup| parameter, and in the NTT_internal() case - by the |type| parameter.

Can you keep ext_pow there for now

I see no reason to. Again, LDE_powers has lg_blowup so what would ext_pow contribute? Indeed, lg_blowup will always be 0 if ext_pow is false, and non-zero if ext_pow is true. In other words, ext_pow is just lg_blowup != 0. And in NTT_internal() there is type that would be normal whenever ext_pow is false, and coset otherwise. Now, let's imagine that we extend NTT kernels implementation to accommodate the expansion internally, for better performance. In this case we would need to pass the lg_blowup factor, which would fully describe the operation. In other words, keeping ext_pow as a placeholder asks for it to be rendered redundant later on.

[dot-asm]

we can replace LDE with LDE_aux.

Done.

[dot-asm]

As a point of clarification in regard to "allegedly" in the following sentence from above:

LDE_spread_distribute_powers kernel is the one where ext_pow is [allededly] meaningful

Judging from the implementation in isolation one can imagine a case when lg_blowup would need to be forced to zero when perform_shift is true. And the question is if this is actually a realistic scenario. Or in other words, is there a situation when you "perform shift" but won't add lg_blowup? Point is that if not, then one doesn't need to add ext_pow to the LDE methods.

Or looking at it from a slightly different angle. As it stands now LDE_spread_distribute_powers with perform_shift set to false is just zero-extension/padding in reverse-bit order. Is it actually the intention? I mean is there a case when you would do just zero-extension in isolation? Because if not, then the implementation doesn't look right. It would make more sense to have something similar to

         index_t pow = bit_rev(idx, lg_domain_size);
         if (perform_shift)
             pow <<= lg_blowup;
        r = r * get_intermediate_root(pow, gen_powers);

[sandsentinel]

LDE_powers has lg_blowup so what would ext_pow contribute?

You're right in the context of LDE_powers. In the context of LDE itself where we call lde_spread_distribute_powers, we still need it.

As it stands now LDE_spread_distribute_powers with perform_shift set to false is just zero-extension/padding in reverse-bit order.

Yes, this is intended. The idea behind is there might be cases where one might want the intermediate data to be also shifted. This would require calling LDE_distribute_powers then LDE_spread_distribute_powers.

Or in other words, is there a situation when you "perform shift" but won't add lg_blowup?

Yes. In fact this is the usual way LDE is done. The exponent usually isn't shifted by lg_blowup.

[dot-asm]

Just in case, I've re-based it. But note that it now performs test-compile :-)

[dot-asm]

LDE_powers has lg_blowup so what would ext_pow contribute?

You're right in the context of LDE_powers. In the context of LDE itself where we call lde_spread_distribute_powers, we still need it.

What do you mean "still"? One of my original points is that it's not exposed to the application, there is no "still." Also note that I didn't touch the flag in LDE_launch()... In other words, this is not an objection to the original suggestion...

[sandsentinel]

What do you mean "still"?

I mean that we still need for LDE_spread_distribute_powers, and the option should be exposed to the application. I agree with removing it from LDE_distribute_powers.

[dot-asm]

I agree with removing it from LDE_distribute_powers.

So there are no objections then. As it stands here and now that is. Yes, LDE_aux needs more work, but it can be done separately...

[sandsentinel]

LDE_aux needs more work, but it can be done separately...

I can do that at a later time.

[dot-asm]

I've squashed [some] commits, could you skim through it one more time, please?