Giter Club home page Giter Club logo

Comments (20)

genotrance avatar genotrance commented on May 24, 2024 1

Am not keen on adding a terminal dependency into cimport at this time. Will reconsider later when I get a chance to evaluate performance and binary size impacts.

Closing.

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024 1

@genotrance Once you cut a new release of nimterop, I plan to update my svdpi wrapper with this change: kaushalmodi/nim-svdpi@57268e7

from nimterop.

zedeus avatar zedeus commented on May 24, 2024

What happens if you run this line?
toast --pnim --preprocess /home/kmodi/sandbox/systemverilog/dpi_c/nim_svdpi/svdpi.h

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Thanks, in that toast output, I found a gcc command, and running that I realized that I had forgotten to put this svdpi_compatibility.h in the path too.

But after adding that too, I get error:

libdpi.nim(5, 8) template/generic instantiation of `cImport` from here
../../../../stow/pkgs/nim/devel/lib/core/macros.nim(525, 20) Error: undeclared identifier: 's_vpi_vecval'

from nimterop.

zedeus avatar zedeus commented on May 24, 2024

Seems like a bug.

After --preprocess:

typedef struct vpi_vecval {
    uint32_t a;
    uint32_t b;
} s_vpi_vecval, *p_vpi_vecval;
typedef s_vpi_vecval svLogicVecVal;

After --pnim:

type
  svLogicVecVal* = s_vpi_vecval

Bug gets triggered by defining multiple structs after the same typedef.

Possibly a tree-sitter bug?

typedef struct foo {
    int i;
} a, b;

Gets translated to:

(translation_unit 1 1 39
 (type_definition 1 1 39
  (struct_specifier 1 9 25
   (type_identifier 1 16 3)
   (field_declaration_list 1 20 14
    (field_declaration 2 5 6
     (primitive_type 2 5 3)
     (field_identifier 2 9 1)
    )
   )
  )
  (ERROR 3 3 2
   (type_identifier 3 3 1)
  )
  (type_identifier 3 6 1)
 )
)

But without the extra name:

(translation_unit 1 1 36
 (type_definition 1 1 36
  (struct_specifier 1 9 25
   (type_identifier 1 16 3)
   (field_declaration_list 1 20 14
    (field_declaration 2 5 6
     (primitive_type 2 5 3)
     (field_identifier 2 9 1)
    )
   )
  )
  (type_identifier 3 3 1)
 )
)

from nimterop.

zedeus avatar zedeus commented on May 24, 2024

Issue addressing the bug here tree-sitter/tree-sitter-cpp#30

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Thanks guys!

It compiles after this workaround:

import strformat
import nimterop/cimport

# Below manual definition of s_vpi_vecval is a workaround for
# https://github.com/genotrance/nimterop/issues/47.
type
  s_vpi_vecval* {.importc: "s_vpi_vecval".} = object
    aval: uint32
    bval: uint32

cImport("svdpi.h")

# Input: none
# Return: none
proc hello() {.exportc.} =
  echo svDpiVersion()
  echo "Hello from Nim!"

from nimterop.

genotrance avatar genotrance commented on May 24, 2024

I'm thinking this can be marked as fixed by providing better error handling when preprocessor complains. Nimterop will now highlight preprocessor errors. It will make it more obvious to users.

We can track the tree-sitter bug in their issue tracker. Let me know if this is acceptable.

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

I'm thinking this can be marked as fixed by providing better error handling when preprocessor complains. Nimterop will now highlight preprocessor errors. It will make it more obvious to users.

That's great! For this issue, I had to run toast to get the gcc command, and then the gcc command to get the actual error. With improved error, will I get to see the error thrown by gcc?

We can track the tree-sitter bug in their issue tracker.

Agreed.

Let me know if this is acceptable.

Yes, it is. Thanks.

from nimterop.

genotrance avatar genotrance commented on May 24, 2024

Yes you should see the error right there inline. Can you please try with this same library?

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Yes, now I get:

st --pnim --preprocess /home/kmodi/sandbox/systemverilog/dpi_c/nim_svdpi/svdpi.h
stack trace: (most recent call last)
../../../../.nimble/pkgs/nimterop-0.1.0/nimterop/cimport.nim(237, 22) cImport
../../../../.nimble/pkgs/nimterop-0.1.0/nimterop/cimport.nim(98, 11) getToast
../../../../stow/pkgs/nim/devel/lib/system.nim(3989, 20) failedAssertImpl
../../../../stow/pkgs/nim/devel/lib/system.nim(3982, 11) raiseAssert
../../../../stow/pkgs/nim/devel/lib/system.nim(3016, 7) sysFatal
libdpi.nim(11, 8) template/generic instantiation of `cImport` from here
../../../../stow/pkgs/nim/devel/lib/system.nim(3016, 7) Error: unhandled exception: /home/kmodi/.nimble/pkgs/nimterop-0.1.0/nimterop/cimport.nim(98, 12) `ret == 0`
ERROR: : svdpi_compatibility.h: No such file or directory

image

Minor nitpick: Can that last ERROR also be in red color as the Error above?

Note that I reproduced the earlier error. Not the last one.

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Should this issue be reopened given that the upstream bug is fixed: tree-sitter/tree-sitter-cpp#30?

Will that fix auto-propagate to nimterop? Or does nimterop need to be updated to absorb that upstream fix?

from nimterop.

genotrance avatar genotrance commented on May 24, 2024

I believe this works in nimterop as of today.

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

I removed this workaround code:

type
  s_vpi_vecval* {.importc: "s_vpi_vecval".} = object
    aval: uint32
    bval: uint32

and updated nimterop from head.

But I still the same error:

/home/kmodi/usr_local/apps/6/nim/devel/lib/core/macros.nim(595, 33) Error: undeclared identifier: 's_vpi_vecval'

Full Nim wrapper

# svdpi.nim
import nimterop/cimport
import os

static:
  cDisableCaching()

const
  xlmIncludePath = getEnv("XCELIUM_ROOT") / ".." / "include"
static:
  doAssert fileExists(xlmIncludePath / "svdpi.h")
  doAssert fileExists(xlmIncludePath / "svdpi_compatibility.h")
  cAddSearchDir(xlmIncludePath)

cDefine("DPI_COMPATIBILITY_VERSION_1800v2012")

# # Below manual definition of s_vpi_vecval is a workaround for
# # https://github.com/nimterop/nimterop/issues/47.
# type
#   s_vpi_vecval* {.importc: "s_vpi_vecval", header: xlmIncludePath / "svdpi.h".} = object
#     aval*: uint32 # we need to export the object elements too!
#     bval*: uint32

cImport(cSearchPath("svdpi.h"))

Snippet from svdpi.h Header file

#ifndef VPI_VECVAL
#define VPI_VECVAL
typedef struct vpi_vecval {
    uint32_t a;
    uint32_t b;
} s_vpi_vecval, *p_vpi_vecval;
#endif

/* (a chunk of) packed logic array */
typedef s_vpi_vecval svLogicVecVal;

Ref

Matching output from cDebug()

It looks like the s_vpi_vecval and *p_vpi_vecval do not get defined at all.

# scalar
#
# * DPI representation of packed arrays.
# * 2-state and 4-state vectors, exactly the same as PLI's avalue/bvalue.
#
# (a chunk of) packed logic array
  svLogicVecVal* {.impsvdpi.} = s_vpi_vecval

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

OK .. what's interesting is that svdpi_compatibility.h is not getting parsed at all.

import nimterop/cimport
import os

static:
  cDisableCaching()
  cDebug()

const
  xlmIncludePath = getEnv("XCELIUM_ROOT") / ".." / "include"
static:
  doAssert fileExists(xlmIncludePath / "svdpi_compatibility.h")
  cAddSearchDir(xlmIncludePath)

cDefine("DPI_COMPATIBILITY_VERSION_1800v2012")
cImport(cSearchPath("svdpi_compatibility.h"))

This outputs:

{.passC: "-DDPI_COMPATIBILITY_VERSION_1800v2012".}

# Importing /masking/the/actual/company/internal/path/svdpi_compatibility.h
# Generated at 2019-05-14T15:38:10-04:00
# Command line:
#   /home/kmodi/.nimble/pkgs/nimterop-#head/nimterop/toast --pnim --preprocess --defines+=DPI_COMPATIBILITY_VERSION_1800v2012 --nim:/home/kmodi/usr_local/apps/6/nim/devel/bin/nim /masking/the/actual/company/internal/path/svdpi_compatibility.h

{.hint[ConvFromXtoItselfNotNeeded]: off.}

import nimterop/types

const
  headersvdpi_compatibility {.used.} = "/masking/the/actual/company/internal/path/svdpi_compatibility.h"

{.pragma: impsvdpi_compatibility, importc, header: headersvdpi_compatibility.}
{.pragma: impsvdpi_compatibilityC, impsvdpi_compatibility, cdecl.}

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Actually, it's the same bug. I can easily reproduce with just this in svdpi_compatibility.h:

typedef struct t_vpi_vecval {
uint32_t aval;
uint32_t bval;
} s_vpi_vecval, *p_vpi_vecval;

With above, I get:

# Generated at 2019-05-14T15:44:52-04:00
# Command line:
#   /home/kmodi/.nimble/pkgs/nimterop-#head/nimterop/toast --pnim --preprocess --defines+=DPI_COMPATIBILITY_VERSION_1800v2012 --nim:/home/kmodi/usr_local/apps/6/nim/devel/bin/nim ../includes/svdpi_compatibility.h

{.hint[ConvFromXtoItselfNotNeeded]: off.}

import nimterop/types

const
  headersvdpi_compatibility {.used.} = "../includes/svdpi_compatibility.h"

{.pragma: impsvdpi_compatibility, importc, header: headersvdpi_compatibility.}
{.pragma: impsvdpi_compatibilityC, impsvdpi_compatibility, cdecl.}

With:

typedef struct t_vpi_vecval {
uint32_t aval;
uint32_t bval;
} s_vpi_vecval;

( I removed the , *p_vpi_vecval portion. )

I get:

# Generated at 2019-05-14T15:46:53-04:00
# Command line:
#   /home/kmodi/.nimble/pkgs/nimterop-#head/nimterop/toast --pnim --preprocess --defines+=DPI_COMPATIBILITY_VERSION_1800v2012 --nim:/home/kmodi/usr_local/apps/6/nim/devel/bin/nim ../includes/svdpi_compatibility.h

{.hint[ConvFromXtoItselfNotNeeded]: off.}

import nimterop/types

const
  headersvdpi_compatibility {.used.} = "../includes/svdpi_compatibility.h"

{.pragma: impsvdpi_compatibility, importc, header: headersvdpi_compatibility.}
{.pragma: impsvdpi_compatibilityC, impsvdpi_compatibility, cdecl.}

type
  t_vpi_vecval* {.importc: "struct t_vpi_vecval", header: headersvdpi_compatibility, bycopy.} = object
    aval*: uint32
    bval*: uint32
  s_vpi_vecval* {.impsvdpi_compatibility, bycopy.} = t_vpi_vecval

from nimterop.

genotrance avatar genotrance commented on May 24, 2024

Can you delete the build folder and rebuild nimterop? You're probably still using the old tree-sitter.

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Can you delete the build folder and rebuild nimterop?

I had already done that. I again did:

nimble uninstall nimterop
nimble install nimterop@#head
nim c svdpi.nim

and I got the same error.

I confirmed the time stamp of the toast binary and the files under ~/.nimble/pkgs/nimterop-#head/nimterop/treesitter/ .. they all are the latest.

You're probably still using the old tree-sitter.

How do I verify the tree-sitter version?

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

Looks like the ~/.nimble/pkgs/nimterop-#head/build is auto-deleted each time after nimble install nimterop@#head. So I should be always fetching the latest tree-sitter.

from nimterop.

kaushalmodi avatar kaushalmodi commented on May 24, 2024

I can reproduce the same issue even after pulling latest nimterop and doing nimble install.

from nimterop.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.