#470 PERFORM INLINE OPTIMIZATIONS

[Denis HUGONNARD-ROCHE]

In a perform varying identifer-1 from identifier-2 when identifier-1 is the same than identifier-2 as below

PERFORM varying FLD-2 FROM FLD-2 BY -1 UNTIL FLD-2 = 0

gnucobol generates the initialization of identifier-1 by itself

  /* Line: 10        : PERFORM            : tstperf.cbl */
  memmove (b_17, b_17, 8);

In this case we could avoid the memmove generation

[Simon Sobisch]

I agree. Do you want to have a look in the code yourself on this or should we inspect together?

[Denis HUGONNARD-ROCHE]

Hi Simon
I will try to find the solution by digging in the code and propose a patch.
I will surely have questions ;-)

[Denis HUGONNARD-ROCHE]

I guess it's there in codegen.c

        case CB_PERFORM_UNTIL:
                v = CB_PERFORM_VARYING (CB_VALUE (p->varying));
                if (v->name) {
                        output_move (v->from, v->name);

We have to check that from and name are different to output_move

[Simon Sobisch]

Yes, in this case we can do that directly here to just not generate that internal MOVE. In the function itself we can do something similar, but only if IBM move is not active (may be handled outside already) and then output a no_op call instead.

[Denis HUGONNARD-ROCHE]

Simon
Here is a patch proposal
It passes all the test (make check , make test)
I add a test program

[Denis HUGONNARD-ROCHE]

The patch

[Simon Sobisch]

Thank you for investigating it! That looks very promising - but you can improve the test (same test) to also check the debug item; and for this to work you need the is_same_reference only around the output_move, not also for the debug handling.

For the new function - just add it to the local functions (no need for the extra declaration) and shorten it:

+static COB_INLINE COB_A_INLINE int
+is_same_reference (cb_tree ref_1, cb_tree ref_2)
+{
+   return CB_REFERENCE (ref_1)->hashval == CB_REFERENCE (ref_2)->hashval;
+}

... but: is that actually correct? I think hashval is not the hash for the reference but the hash-entry for the word-table, no?

In this case the check has to be made more complicated (maybe getting field founder and offset + size [see overlapping check in typeck.c])? Doing that would also have the benefit of not doing a move if there was a simple redefine; but then again we'd need to check for the same usage as well.

I do wonder if we should add that inline (if it is small) function also in typeck.c cb_build_move_copy() and use this for all but the cb_move_ibm case.

[Denis HUGONNARD-ROCHE]

Hi Simon

Here is a second patch proposal including the associated tests.
This patch allows the compiler not to generate a move if and only the name of the two variables are identical
All tests are ok (make check and make test)

In a second I would try to deal with the fact that both variables are "redefines", but I need to increase my level of knowledge of GnuCobol for that ;-)

But the current patch allows, in my opinion, to cover the majority of cases

[Simon Sobisch]

Thank you for this second iteration.

Please adjust the test title - "DHR" is not that well, possibly just "MOVE to same field"?
Is there a reason to not have a single test? Each test can also contain multiple programs (prog1.cob, prog2.cob, ....). I think the DEBUG-ITEM test (please add that in the keywords) can be combined with another one - either you have COB_SET_DEBUG=1 set or not.
Please add a short comment (either in COBOL or with # outside) what is special about each test program (at least from a very quick glance it wasn't easy to spot).
If you really want to test that the moves are not generated: add --save-temps to the compile command, then $GREP for all moves (or similar).

For adjusting this place, there is no need for cob_nop as we still (should) have the codegen of the statement. So something like

            /* no use in generating move for PERFORM VARYING VAR FROM VAR */
            if ( !is_same_var (v->from, v->name)) {
                output_move (v->from, v->name);
            }

For is_same_var: that possibly breaks for FROM level78constant or FROM FUNCTION NUMVAL (VAR).
Best seems to be something like:

*/ may not be necessary if all inputs are a reference *
/
if (!CB_REFERENCE_P (var_1) || CB_REFERENCE_P (var_2)) return 0;
ref1 = CB_REFERENCE_P (var_1);
ref2 = CB_REFERENCE_P (var_2);

then comparing if the ref->value (= the COBOL field) as well as the subs are identical. The current approach ignores VAR1 (A) FROM VAR1(B). In other places (like the normal MOVE) we'd have to also check the offset and length, but we only allow numeric items here so that doesn't apply (if you rename the function to something like is_same_numeric_reference)

[Simon Sobisch]

friendly ping @dhugonnard

[Denis HUGONNARD-ROCHE]

Hi Simon
I have been very busy with my job and also wanted to find a solution that really satisfied me for the cob_decimal_pow function. The previous patch that I sent wasn't yet good enough for my taste.
I think I achieved what I wanted for this feature.
Attached you will find the patch which contains the source code as well as all the associated test programs
The principles of the modifications are as follows:
0) to calculate a^b we immediately transform a into mpt_t type to use the mpf_pow_ui function
1) before calculating a^b, we check that b*ln(a) is not greater than 38. If the result is greater than 38 we raise an exception to generate a 'SIZE ERROR'.
3) for a^b if b is negative and integer we calculate the absolute value of b and 1/a THEN we test if 'abs(b) fits_ulong'.

I was also interested in the optimization of logarithm calculations by creating a proof concept to calculate ln with the CORDIC algorithm.
You will find the test program in the attachment.

To use it :

1) modify the visibility of cob_mpf_log in libcob so as not to declare it 'static'
2) compile by 'cobc -x cob_cordic.c -lgmp'
3) run the program by ./cob_cordic_test >result_cob 2> result_cordic
4) diff result_cob result_cordic.
This algorithm is interesting in the case where we know the precision we need.

I will now work again on optimization #470
Dennis

[Denis HUGONNARD-ROCHE]

OK i'll do that this week
Denis