Draft: Performance improvement around recursive definition of if statements

[QuantamHD]

@alaindargelas I have another performance improvement suggestion around the generate ifs. In the current version of the grammar it relies on a recursive definition of if else. After profiling Surelog and ANTLR I've found that these recursive definitions kill performance. Each level of rule indirection that is calling one rule from another is a very expensive operation in ANTLR. The generate ifs in the Google test case have over 100,000 if else if branches which makes this file pretty slow.

In this PR I propose replacing the overly general grammar with a more tailored grammar that reduces the long rule chains that hurt performance. This will require a change in Surelog's ParseTree to AST code which I will take a look at, but if you can beat me to it @alaindargelas that would be ideal.

The grammar now relies on ()* which doesn't require adding a new context to the stack in ANTLR.

Test Case

module conditional_generate
    #(parameter OPERATION_TYPE = 0)
    (
        input  logic [31:0] a,
        input  logic [31:0] b,
        output logic [63:0] z
    );

    // The generate-endgenerate keywords are optional.
    // It is the act of doing a conditional operation
    // on a parameter that makes this a generate block.
    generate
        if (OPERATION_TYPE == 0) begin
            assign z = a + b;
        end
        else if (OPERATION_TYPE == 1) begin
            assign z = a - b;
        end
        else if (OPERATION_TYPE == 2) begin
            assign z = (a << 1) + b; // 2a+b
        end
        ... // Add a hundred thousand of these else if cases to match the google case
        else begin
            assign z = b - a;
        end
    endgenerate
endmodule: conditional_generate

Bad Profile

Old Version

New Version

[alaindargelas]

@QuantamHD, Let me work on adapting Surelog to this grammar, it is pretty involved, but first the new grammar is not complete,
please check all the syntax errors it creates in the regression. That has to be fixed and is purely a function of the grammar.

[alaindargelas]

Locally on your machine, after changing the grammar, run:

rm -rf build/bin/pkg
make regression CPU_CORES=8

to have fast turnaround.

[Thomasb81]

Could a similar method can be use to solve : #2035 ?

[QuantamHD]

@Thomasb81 It could help. It would be helpful to post what constructs you suspect are contributing to the expensive evaluation in black parrot.

[alaindargelas]

The file is attached in the issue, it is the regular if-else-if (Not the if-generate-else-if)

 if (~reset_i & trace_en_i) begin
          trace_fd = $fopen(tracefile_lp, "a");

          // If miss handler has finished the dma request and result is ready
          // for a missed request
          if (inc_miss_ld)
            print_operation_trace(trace_fd, my_name, "miss_ld");
          else if (inc_miss_st)
            print_operation_trace(trace_fd, my_name, "miss_st");


          // If miss handler is still busy on a request
          else if (miss_v) begin
            print_operation_trace(trace_fd, my_name, "miss");
          end 

        
          // If response is ready for a hit request
          else begin

            if (inc_ld) begin
              if (inc_ld_ld) 
                print_operation_trace(trace_fd, my_name, "ld_ld");
              else if (inc_ld_ldu)
                print_operation_trace(trace_fd, my_name, "ld_ldu");
              else if (inc_ld_lw) 

[QuantamHD]

@alaindargelas I would stop working on adapting the grammars if you started. After I fixed the syntax issues I saw worse performance than master. I'll have to give this a bit more thought

[alaindargelas]

@QuantamHD, I'm not working on this. If you find a faster legal grammar, then I'll adapt Surelog to use that grammar.

[Thomasb81]

As @alaindargelas mentioned #2035 contain a testcase, that syntactically describe a cascade of "if else if" that is some how drawing the performance. At first glance it look very similar to the grammar part you address except it is not in a generate.

Antlr literature discuss performance issue related to ambiguity but in this case tool is not reporting ambiguity. So any finding is very interesting.

@alaindargelas Have you any method to only regress the grammar using existing testsuite but without having to update Surolg to grammar change but still having pre-processor wokring ? because realistic testcase often need preprocessor.
Quickly grammar trial and change break Surolog. Additionnally working on realistic usecase with SV3_1a grammar and the java antlr runtime is quickly difficult

I should admit that not having this capability at the time I try to contribute on #2035 has probably contribute to my give up.

[alaindargelas]

@Thomasb81, unfortunately, the fastest way is:

• Edit the grammar
• cd Surelog
• rm -rf build/bin/pkg
• make regression CPU_CORES=8

That runs in about 5 mins clock wall time on an 8 cores machine, including build time and whole regression.

[alaindargelas]

@QuantamHD, shall we close this?