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

type info is being repeated in some cases #46

Closed
mayagokhale opened this issue Feb 16, 2020 · 10 comments
Closed

type info is being repeated in some cases #46

mayagokhale opened this issue Feb 16, 2020 · 10 comments
Assignees
Labels

Comments

@mayagokhale
Copy link
Collaborator

This is a problem that I thought was previously fixed, but noticed it again on the current master branch. Using the test.cpp example from the sc_zfp version the causes find_emax to be instantiated, certain type parameters are being repeated. So for the signal "data" in stream.h
sc_signal data;

we see the following generated .txt fragment below. Additionally, the sc_stream_0 module is being emitted three times. I wonder if this could be because the matcher is matching multiple rules and adding the matched item to the list each time.

hModule sc_stream_0 [
hPortsigvarlist NONAME [
hSigdecl data NOLIST
hTypeinfo NONAME [
hType sc_signal NOLIST
hType fp_t<11, 52> NOLIST
hType fp_t<11, 52> NOLIST
]
hSigdecl ready NOLIST
hTypeinfo NONAME [
hType sc_signal NOLIST
hType _Bool NOLIST
]
hSigdecl valid NOLIST
hTypeinfo NONAME [
hType sc_signal NOLIST
hType _Bool NOLIST
]
]

@rseac rseac self-assigned this Feb 17, 2020
@rseac rseac added the bug label Feb 17, 2020
@rseac
Copy link
Collaborator

rseac commented Feb 17, 2020

@mayagokhale There are two bugs this issue identifies that have been noted.

  1. The type fp_t<11,5> does not get parsed further down.
  2. The duplicate template arguments of fp_t<11,5>

For (1), although not entirely sure, I suspect that the RecursiveASTVisitor does not completely visit all children of template instantiations. This is perhaps one of the reasons why such issues are coming up. I'm going to try first by updating clang to see if some of those have been resolved. If not, then something else must be devised.

For (2), this should have been fixed in the master (d303ef5).

The concern about sc_stream_0 being emitted multiple times still needs further investigation. I'll need to create smaller examples to isolate this.

@mayagokhale
Copy link
Collaborator Author

I confirmed that 2 is fixed.

@rseac
Copy link
Collaborator

rseac commented Feb 21, 2020

Issue (1) occurs within sc_stream, which defines an sc_signal<T> data with T=fp_t<11,52>. The parsing fails because this is recognized as a RecordType at some point in the hierarchy. The way to access the underlying ClassTemplateSpecialization is to access the ClassTemplateSpecializationDecl via a dyn_cast, and then find the template arguments.

Related:

  1. What should one do with template arguments such as SC_CORE::SC_ONE_WRITER which is defined to 0?
  2. How should the templates really be representated when there are nested types that themselves will have multiple template arguments? A simple list would not work.

side:
Interestingly, if the declaration of sc_signal<fp_t<11,52>> data is made explicitly without it being inside a templated sc_module, then the AST representation is ever so slightly different causing the parsing to work differently ...

@rseac
Copy link
Collaborator

rseac commented Mar 3, 2020

@mayagokhale A complete rewrite of FindTemplateTypes in #58 should address (1) and (2).

@rseac
Copy link
Collaborator

rseac commented Mar 19, 2020

This seems to be working fine afaik.

@rseac rseac closed this as completed Mar 19, 2020
@mayagokhale
Copy link
Collaborator Author

In the latest build, type information appears to be duplicated again, this time the template type parameters. As a simple instance, the signal clk which is a sc_in, shows up twice. This is true of all the types being reported from the port, signal, and variable lists.

object name is clk
number of template type args is 2
RecordType 0x10742b920 'class sc_core::sc_in<_Bool>'
-ClassTemplateSpecialization 0x10786f298 'sc_in' RecordType 0x10742b920 'class sc_core::sc_in<_Bool>' -ClassTemplateSpecialization 0x10786f298 'sc_in'

@mayagokhale mayagokhale reopened this Mar 30, 2020
@rseac
Copy link
Collaborator

rseac commented Mar 30, 2020

@mayagokhale I don't see this in any of my tests. For example, tests/t1.cpp in line lines 145 to 151 explicitly test for the number of ports found (input, output, etc.). Furthermore, when printing it out to the console, I also do not see it.

Can you point me to how you are accessing the port information in your code?

@mayagokhale
Copy link
Collaborator Author

In plugins/xlat/Xlat.cpp, line 114 is the function that emits type declarations to hcode and prints debug information. Line 121 prints out the number of template type arguments, and then the code iterates through the arguments and dumps each argument.
In the debug output it prints for example
number of template type args is 2
RecordType 0x109472920 'class sc_core::sc_in<_Bool>'
-ClassTemplateSpecialization 0x1098bd298 'sc_in' RecordType 0x109472920 'class sc_core::sc_in<_Bool>' -ClassTemplateSpecialization 0x1098bd298 'sc_in'

@rseac
Copy link
Collaborator

rseac commented Mar 30, 2020

@mayagokhale I understand the issue.

After the introduction of Tree to represent the types, the way to access the internal types is different. The methods used in Xlat.cpp at the moment should be marked as deprecated, and I'll work on removing them.

To give you an example on how to access the internal types, I've updated the t1.cpp example. The Tree class implements iterators that use depth-first traversal to access the types. #82 includes the updated example.

The code fragment that is important is below.

    for (auto const &port : test_module_inst->getIPorts()) {
      auto name{get<0>(port)};
      auto template_args{template_type->getTemplateArgTreePtr()};

      // Note: template_args must be dereferenced.
      for (auto const &node : *template_args) {
        auto type_data{node->getDataPtr()};
        llvm::outs() << "\n@> name: " << name << ", type name: " << type_data->getTypeName() << " ";
      }
      llvm::outs() << "\n";
   }

Things to note:

  1. getTemplateArgTreePtr() gets the Tree. This is something you were already doing.
  2. for (auto const &node : *template_args) steps through each of the types in the tree using depth-first traversal. Note: we must dereference template_args here. Hope this helps.

@mayagokhale
Copy link
Collaborator Author

Thank you I have updated the Xlat.cpp code and confirms that it is now correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants