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

Functions (Part 1) #470

Merged
merged 44 commits into from
Jun 29, 2023
Merged

Functions (Part 1) #470

merged 44 commits into from
Jun 29, 2023

Conversation

ThomasHaas
Copy link
Collaborator

@ThomasHaas ThomasHaas commented Jun 22, 2023

This PR introduces a Function class. Events are now part of a Function rather than a Thread.
However, a Thread inherits from Function, so events can be part of a thread if the function happens to be a thread.

While adding/parsing functions, I noticed that the boogie parser is a nightmare I needed to fix first. So most of this PR is now dedicated to cleaning up VisitorBoogie and its related components.
An overview of the changes:

  • VisitorBoogie and its related components are far simpler now
  • VisitorBoogie can parse in inline mode and no-inline mode (the former mode matches with what we are doing right now)
  • The parser currently runs once in inline mode and once without, generating proper functions rather than threads.
    However, the generated functions are incomplete (contain placeholders) and unused in this PR. A future PR (Part 2?) will change this. For the purpose of reviewing this PR, code inside !inlineMode-conditionals can be disregarded and parts marked by "// TODO ---- Test Code ----" can be assumed to be commented out (those are for the future PR).
  • Issue Issues with Duplicated Labels #260 should be fixed.
  • Scope-prefixes reset per thread now, i.e., they start from 0: per thread. This was previously not the case because of the above issue with duplicate label names.
  • The parser creates no more unused dummy registers in functions/threads.

Added Program.getFunctions() (unused for now)
Added Return event
Moved all function related code into its own package (only temporarily)
Generalized Function to take parameter names as input and generate "parameter registers".
Added LlvmCmpXchg.setStructRegister (special case)
Added missing transformExpressions implementation to lisa.RMW
Generalized Function.append
Added getScopedName(String) to VisitorBoogie
Made use of VisitorBoogie.addEvent when possible.
# Conflicts:
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/LkmmProcedures.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/LlvmProcedures.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/VisitorBoogie.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/analysis/Dependency.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/analysis/alias/EqualityAliasAnalysis.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/arch/lisa/RMW.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/arch/ptx/PTXFenceWithId.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/arch/ptx/RedOp.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/arch/tso/Xchg.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/RMWAbstract.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/catomic/AtomicAbstract.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/catomic/AtomicCmpXchg.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/linux/LKMMLoad.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/linux/LKMMStore.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/linux/RMWAddUnless.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/linux/RMWCmpXchg.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/llvm/LlvmAbstractRMW.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/event/lang/llvm/LlvmCmpXchg.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/LoopUnrolling.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/Simplifier.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorArm8.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorBase.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorC11.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorIMM.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorLKMM.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorPTX.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorPower.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorRISCV.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/specification/AssertBasic.java
Made label-tracking context-aware so that different threads can have labels with the same name.
@ThomasHaas ThomasHaas changed the title [DRAFT] Functions [DRAFT] Functions (Part 1) Jun 27, 2023
Refactor/renaming
Moved handling of abort() and reach_error() into StdProcedures/SvcompProcedures
Functions now get proper ids.
Updated printer to include function/thread ids in the output.
@ThomasHaas ThomasHaas changed the title [DRAFT] Functions (Part 1) Functions (Part 1) Jun 28, 2023
# Conflicts:
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/boogie/LlvmFunctions.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/boogie/SmackPredicates.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusAArch64.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/LkmmProcedures.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/PthreadsProcedures.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/StdProcedures.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/SvcompProcedures.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/boogie/VisitorBoogie.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/Register.java
#	dartagnan/src/main/java/com/dat3m/dartagnan/program/Thread.java
#	dartagnan/src/test/java/com/dat3m/dartagnan/miscellaneous/AnalysisTest.java
public void initThread(String name, int id){
if(!threads.containsKey(id)){
public void initThread(String name, int id) {
if(!functions.containsKey(id)){
Skip threadEntry = EventFactory.newSkip();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry sounds like a better name since this is not necessary a thread.

Not sure if it make sense in the current state of this PR, but shouldn't we at some point add FunctionEntry and FunctionExit nodes (i.e. events)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry sounds like a better name since this is not necessary a thread.

It is necessarily a thread that gets created. There is a separate initFunction method that creates functions.

Not sure if it make sense in the current state of this PR, but shouldn't we at some point add FunctionEntry and FunctionExit nodes (i.e. events)?

Why? I don't think functions need any of that, because (1) you can have a empty (= no body) functions and (2) we have a Return event to exit functions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a CFG you would normally have entry/exit nodes. The Return event is exactly what I had in mind (I wrote the comment before going over all changes). Not sure if having a specialized Entry extends Skip would make sense or bring any advantage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, all function bodies start with a $bb0 label. And I think this will also holds true for LLVM code.
At least for now, a dedicated entry node just has disadvantages if you allow for empty-bodied functions. Because on an empty function, f.appendEvent(e) would need to implicitly create an entry event before appending e.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now that I think about it, we will probably need a marker of sorts to distinguish between "empty body" and "no body". Maybe a dedicated FunctionEntry event could serve this purpose. But I would do this in a PR that does add proper support for functions (i.e. part 2 or 3, depending on how difficult it gets :)).

@hernanponcedeleon
Copy link
Owner

LGTM

@ThomasHaas ThomasHaas merged commit f6cd58a into development_rework Jun 29, 2023
1 check passed
@ThomasHaas ThomasHaas deleted the functions branch June 29, 2023 16:59
hernanponcedeleon added a commit that referenced this pull request Jul 15, 2023
Add proper functions #470
Generic Registers #471
Rmw base classes #475
Type Safety #478
Parsing & thread creation #479
Unit tests for thread creation #481
Move memory initialization, inlining, and thread creation from the parser to processing passes #483
Inlining of nondet_int #485
Various fixes/cleanup #487

---------

Co-authored-by: Thomas Haas <[email protected]>
Co-authored-by: René Pascal Maseli <[email protected]>
Co-authored-by: Hernan Ponce de Leon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants