Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to cast non-static DPI export functions to (void *) #1391

Closed
veripoolbot opened this issue Jan 12, 2019 · 11 comments
Closed

Trying to cast non-static DPI export functions to (void *) #1391

veripoolbot opened this issue Jan 12, 2019 · 11 comments
Assignees

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Jan 12, 2019


Author Name: Stan Sokorac
Original Redmine Issue: 1391 from https://www.veripool.org

Original Assignee: Wilson Snyder (@wsnyder)


I've been using about a year old version of Verilator, and when I try to switch to the latest stable, I'm running into this problem...

The design__Syms.cpp file contains a number of "exportInsert" calls for each of the DPI functions that I have scattered all over the design:

	__Vscope_path_to_my_block.exportInsert(__Vfinal,"my_dpi_function", (void*)(&my_block::__Vdpiexp_path_to_my_block));@

There's a few hundred of these, and each of these lines is trying to cast the address of a function to (void *) pointer. I run into compile problems here because some of these functions it is trying to cast are static, and those are fine to get an address of like this, and some are not static and can't be cast to (void *) without an instance of the object. I can't figure out why Verilator would generate some of these as static and some not... Every one of my DPI functions is the same, auto-generated code that looks like:

 export "DPI-C" function my_dpi_function_name;
 function int my_dpi_function_name() ;
     return signal_value ; 
  endfunction 

I can't see any pattern in which ones are chosen to be static and which are not....

In the ~1 year old version that I was previously using, I've checked that all DPI functions were generated as static member functions, and hence I had no compile problems.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 12, 2019


Original Redmine Comment
Author Name: Stan Sokorac
Original Date: 2019-01-12T02:44:44Z


I should also add that just hacking the code to add 'static' to all dpi declarations doesn't work, as they really aren't static.. they use this->__PVT__signal to get a signal value, while the static ones use vlSymsp->path.__PVT__signal to get to it, where vlSymsp is a parameter passed to them.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 12, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-01-12T14:43:38Z


It seems like the internal isStatic is getting not set or lost on the bad functions for some reason. I don't see anything obvious in the code. Please run with --debug and look for "CFUNC.*STATIC" in the .tree files. Either they won't get created on the bad ones or will get lost; the file in which this happens will indicate where to look.

You'll need the git version, or apply this patch:

diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp
index 99a1df34..1d3888a6 100644
--- a/src/V3AstNodes.cpp
+++ b/src/V3AstNodes.cpp
@@ -1197,6 +1197,7 @@ void AstCFunc::dump(std::ostream& str) {
      this->AstNode::dump(str);
      if (slow()) str<<" [SLOW]";
      if (pure()) str<<" [PURE]";
+    if (isStatic()) str<<" [STATIC]";
      if (dpiImport()) str<<" [DPII]";
      if (dpiExport()) str<<" [DPIX]";
      if (dpiExportWrapper()) str<<" [DPIXWR]";

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 12, 2019


Original Redmine Comment
Author Name: Stan Sokorac
Original Date: 2019-01-12T18:41:31Z


It looks like step 061, "descope", is the one that lost the [STATIC]. design_060_trace.tree shows "[STATIC] [DPIX]" for the bad function, and design_061_descope.tree shows "[DPIX]" only.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 13, 2019


Original Redmine Comment
Author Name: Eric Mejdrich
Original Date: 2019-01-13T18:13:52Z


Apologies for jumping on to this thread, but would this be the same as this issue? I now see this when I have a submodule (that has exported DPI functions) that is instantiated multiple times in the next level up of hierarchy. Think instantiated ways of a cache and each way is an instantiated array and each array has its own DPI. Apologies for hi-jacking if its something different...

export "DPI-C" function dpi_l2c_dir_array_get_entry;

function void dpi_l2c_dir_array_get_entry(int i, output bit [pWidth-1:0] rval);
    rval = array_q[i];
endfunction
</code>

Vl2c__Syms.cpp:148:117: error: cannot cast from type 'void (Vl2c_l2c_dir_array__pi1::*)(Vl2c__Syms *, const IData, IData &)' to pointer type 'void *'
  ...(void*)(&Vl2c_l2c_dir_array__pi1::__Vdpiexp_dpi_l2c_dir_array_get_entry_TOP__l2c__l2c_directory__gen_ways__BRA__0__KET____DOT__dir_way));
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Vl2c__Syms.cpp:149:117: error: cannot cast from type 'void (Vl2c_l2c_dir_array__pi1::*)(Vl2c__Syms *, const IData, IData &)' to pointer type 'void *'
  ...(void*)(&Vl2c_l2c_dir_array__pi1::__Vdpiexp_dpi_l2c_dir_array_get_entry_TOP__l2c__l2c_directory__gen_ways__BRA__1__KET____DOT__dir_way));
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Vl2c__Syms.cpp:150:117: error: cannot cast from type 'void (Vl2c_l2c_dir_array__pi1::*)(Vl2c__Syms *, const IData, IData &)' to pointer type 'void *'
  ...(void*)(&Vl2c_l2c_dir_array__pi1::__Vdpiexp_dpi_l2c_dir_array_get_entry_TOP__l2c__l2c_directory__gen_ways__BRA__2__KET____DOT__dir_way));
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Vl2c__Syms.cpp:151:117: error: cannot cast from type 'void (Vl2c_l2c_dir_array__pi1::*)(Vl2c__Syms *, const IData, IData &)' to pointer type 'void *'
  ...(void*)(&Vl2c_l2c_dir_array__pi1::__Vdpiexp_dpi_l2c_dir_array_get_entry_TOP__l2c__l2c_directory__gen_ways__BRA__3__KET____DOT__dir_way));
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4 errors generated.

</code>
@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 13, 2019


Original Redmine Comment
Author Name: Stan Sokorac
Original Date: 2019-01-13T22:44:40Z


Yeah, that's exactly the problem.

I thought it was as simple as "if you have multiple instances of a block that has DPI exports", but when I tried to reproduce it with a simple testcase, verilator correctly generated separate static DPI calls for each of the instances. It seems to be some kind of a corner-case optimization in the 'descope' step.

I've tried to randomly thrown in some "if !isDpi()" into the V3Descope.cpp to avoid setting m_needThis to true (which turns off the static qualifier), but that caused all kinds of havoc :). I think I'll have to leave it to Wilson to figure it out.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 14, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-01-14T01:57:44Z


Thanks for the additional sighting, now I know the module and think that inlining is involved, with these hints I'll try to make a test case & fix.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 14, 2019


Original Redmine Comment
Author Name: Eric Mejdrich
Original Date: 2019-01-14T15:09:22Z


No problem, and thanks for looking at this. Apologies, I don't know the internals anywhere near enough to go digging. In this case I was able to work around by commenting out the DPI export in the leaf module, and moving it up a level and using a case statement switched with the way - so essentially explicitly selecting the array elements per way that I want to access versus setting the scope to the individual array and using the leaf module DPI. This is somewhat painful as the number of ways is parameterizable and the DPI is now essentially hardcoded to a single configuration, and in some cases there are multiple levels of heirarchy that can be involved.

l2_directory_dpi.svh:

export "DPI-C" function dpi_l2c_dir_get_entry;

function void dpi_l2c_dir_get_entry(int way, int i, output bit [$bits(l2c_pkg::dir_entry_t)-1:0] rval);
    case(way) 
       0: rval = gen_ways[0].dir_way.array_q[i];
       1: rval = gen_ways[1].dir_way.array_q[i];
       2: rval = gen_ways[2].dir_way.array_q[i];
       3: rval = gen_ways[3].dir_way.array_q[i];
       default: rval = 'b0;
    endcase
endfunction
</code>

snippet from l2_directory.sv:



`ifdef SIMULATION
`include "l2c_directory_dpi.svh"
`endif

    //-- directory arrays
    generate
       for(a=0; a<l2c_pkg::p_L2C_ASSOCIATIVITY; a++) begin : gen_ways
          l2c_dir_array #(
             .pWidth($bits(l2c_pkg::dir_entry_t)), 
             .pDepth(l2c_pkg::p_L2C_NUM_LINES/l2c_pkg::p_L2C_ASSOCIATIVITY), 
             .pRetimeRd(1),
             .pInitVal( {1'b0, 3'b001, {(l2c_pkg::p_L2C_TAG_ADDR_HI-l2c_pkg::p_L2C_TAG_ADDR_LO+1){1'b0}}} )
          ) dir_way(
             .cnr_clk1x(cnr_clk1x),
             .cnr_reset(cnr_reset),
             .addr(dir_handler_dir.row_addr),
             .we(dir_handler_dir.write),
             .wd(dir_handler_dir.write_set.entry[a]),
             .rd(l2c4_read_set.entry[a])
          );
        end
     endgenerate

</code>
@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 16, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-01-16T05:42:46Z


Fixed in git towards 4.010. Please give it a try.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 16, 2019


Original Redmine Comment
Author Name: Eric Mejdrich
Original Date: 2019-01-16T15:31:02Z


This seems to fix what I was seeing. I will undo some additional workarounds later today and check as well. Thanks much.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 16, 2019


Original Redmine Comment
Author Name: Stan Sokorac
Original Date: 2019-01-16T15:47:17Z


I can confirm that it works for me, too. Thank you!

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 28, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-01-28T12:32:40Z


In 4.010.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.