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

Handle preExp and varDecls for crefs with subs. #923

Merged

Conversation

mahge
Copy link
Contributor

@mahge mahge commented Jun 2, 2020

  • This fixes ticket:5994.

  • We used to throw away any extra expressions and variable declarations
    needed for crefs with (complicated) subscripts, i.e., if it has some complicated
    subscript that can not be generated inline.

  • If you are sure you have a path (a cref with no subs) then you can use
    contextCrefNoPrevExp (e.g. variable names are just paths. They are just
    represented as cref). Otherwise contextCref now needs a preExp and varDecls
    buffer passed to it.

  • Avoid unnecessary code generation.

    • The path we took for crefs with subscripts in function context
      used to create unnecessary temporaries and expressions which we did
      not notice because they were were thrown away after being created.

    • Split up the LHS cref generation template function to take different paths for
      normal and parallel functions.

  • Add a test case for Ticket:5994.

  • Remaining issues

    • There are still cases where cref code generation does not get all needed information
      like buffers and correct context. This needs to be updated.
    • Generation of LHS crefs for parmodelica parallel functions needs to be updated
      for recent changes.
    • contextCrefOld template function needs to be removed after replacing its uses by
      contextCref or an appropriate function.

  - We used to throw away any extra expressions and variable declarations
    needed for it, i.e., if it has some complicated subscript that can not
    be generated inline.

  - If you are sure you have a path (a cref with no subs) then you can use
    contextCrefNoPrevExp (e.g variable names are just paths. They are just
    represented as cref). Otherwise contextCref now needs a preEx and varDecls
    buffer passed to it.
@sjoelund
Copy link
Member

sjoelund commented Jun 2, 2020

Add a reference to ticket:5994 in a commit message

@mahge
Copy link
Contributor Author

mahge commented Jun 2, 2020

@sjoelund Will do. This is not over yet. I am trying to see if these changes (which are mostly renaming and parameter additions) broke anything. Just to to simplify things.

The actual fix comes on the next commit/s

mahge added 3 commits June 2, 2020 15:29
  - This fixes ticket:5994.

  - The path we took for crefs with subscripts in function context
    used to create unnecessary temporaries and exps which we did
    not notice because they were were thrown away after being created.

  - Split up the function to take different paths for normal and parallel
    functions.
@mahge mahge force-pushed the fix_prev_exp_var_decls_for_cref_subs branch from dc70d57 to b93569f Compare June 3, 2020 08:56
@lochel lochel merged commit cc6d66f into OpenModelica:master Jun 3, 2020
adrpo pushed a commit to adrpo/OpenModelica that referenced this pull request Jun 17, 2020
* Handle preExp and varDecls for crefs with subs.

  - We used to throw away any extra expressions and variable declarations
    needed for it, i.e., if it has some complicated subscript that can not
    be generated inline.

  - If you are sure you have a path (a cref with no subs) then you can use
    contextCrefNoPrevExp (e.g variable names are just paths. They are just
    represented as cref). Otherwise contextCref now needs a preEx and varDecls
    buffer passed to it.

* Fix wrong ordering of buffer arguments.

* Avoid unnecessary code generation.

  - This fixes ticket:5994.

  - The path we took for crefs with subscripts in function context
    used to create unnecessary temporaries and exps which we did
    not notice because they were were thrown away after being created.

  - Split up the function to take different paths for normal and parallel
    functions.

* Add a test case for Ticket:5994.
adrpo pushed a commit that referenced this pull request Jun 17, 2020
* Handle preExp and varDecls for crefs with subs.

  - We used to throw away any extra expressions and variable declarations
    needed for it, i.e., if it has some complicated subscript that can not
    be generated inline.

  - If you are sure you have a path (a cref with no subs) then you can use
    contextCrefNoPrevExp (e.g variable names are just paths. They are just
    represented as cref). Otherwise contextCref now needs a preEx and varDecls
    buffer passed to it.

* Fix wrong ordering of buffer arguments.

* Avoid unnecessary code generation.

  - This fixes ticket:5994.

  - The path we took for crefs with subscripts in function context
    used to create unnecessary temporaries and exps which we did
    not notice because they were were thrown away after being created.

  - Split up the function to take different paths for normal and parallel
    functions.

* Add a test case for Ticket:5994.
adrpo pushed a commit to adrpo/OpenModelica that referenced this pull request Sep 28, 2020
* Handle preExp and varDecls for crefs with subs.

  - We used to throw away any extra expressions and variable declarations
    needed for it, i.e., if it has some complicated subscript that can not
    be generated inline.

  - If you are sure you have a path (a cref with no subs) then you can use
    contextCrefNoPrevExp (e.g variable names are just paths. They are just
    represented as cref). Otherwise contextCref now needs a preEx and varDecls
    buffer passed to it.

* Fix wrong ordering of buffer arguments.

* Avoid unnecessary code generation.

  - This fixes ticket:5994.

  - The path we took for crefs with subscripts in function context
    used to create unnecessary temporaries and exps which we did
    not notice because they were were thrown away after being created.

  - Split up the function to take different paths for normal and parallel
    functions.

* Add a test case for Ticket:5994.
adrpo pushed a commit that referenced this pull request Sep 28, 2020
* Handle preExp and varDecls for crefs with subs.

  - We used to throw away any extra expressions and variable declarations
    needed for it, i.e., if it has some complicated subscript that can not
    be generated inline.

  - If you are sure you have a path (a cref with no subs) then you can use
    contextCrefNoPrevExp (e.g variable names are just paths. They are just
    represented as cref). Otherwise contextCref now needs a preEx and varDecls
    buffer passed to it.

* Fix wrong ordering of buffer arguments.

* Avoid unnecessary code generation.

  - This fixes ticket:5994.

  - The path we took for crefs with subscripts in function context
    used to create unnecessary temporaries and exps which we did
    not notice because they were were thrown away after being created.

  - Split up the function to take different paths for normal and parallel
    functions.

* Add a test case for Ticket:5994.
@mahge mahge deleted the fix_prev_exp_var_decls_for_cref_subs branch October 9, 2020 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants