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.

Sign up for GitHub

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

Jump to bottom

DLIR to Souffle Emitter #278

Merged
merged 18 commits into from
Feb 11, 2022
Merged

DLIR to Souffle Emitter #278

merged 18 commits into from
Feb 11, 2022

Conversation

aferr
Copy link
Collaborator

@aferr aferr commented Jan 24, 2022

This implements a pass that generates a Seouffle program as a string from a DLIR program. This pass also generates just the necessarily declarations for the predicates that are used.

@aferr
Copy link
Collaborator Author

aferr commented Jan 24, 2022

I might also take a look at using std::transform and std::accumulate for some of this.

@aferr
Copy link
Collaborator Author

aferr commented Jan 24, 2022

Actually, scratch that thought about trying std::transform and std::accumulate. It turns out std::accumulate and std::reduce are C++20 features and AFAICT std::transform has neither an aesthetic or performance improvement in this case.

@aferr
Copy link
Collaborator Author

aferr commented Jan 24, 2022

Tests are not there yet, but because of the timezone difference, it could be useful to get early review on this.

@aferr aferr requested review from bgogul and markww January 24, 2022 16:57
markww
markww reviewed Jan 24, 2022
View reviewed changes
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/ast.h Outdated Show resolved Hide resolved
src/ir/auth_logic/ast.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
@aferr aferr force-pushed the datalog_ir_emitter@aferr branch 3 times, most recently from 221da59 to 5709b36 Compare January 25, 2022 13:55
@aferr aferr requested a review from markww February 8, 2022 18:12
markww
markww reviewed Feb 9, 2022
View reviewed changes
src/ir/auth_logic/ast.h Outdated Show resolved Hide resolved
src/ir/auth_logic/BUILD Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
@aferr aferr requested a review from markww February 9, 2022 14:55
markww
markww reviewed Feb 9, 2022
View reviewed changes
Copy link
Contributor

@markww markww left a comment

Choose a reason for hiding this comment

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

Looks good mostly, but my big question is why you're using a btree_set instead of flat_hash_set.

src/ir/auth_logic/souffle_emitter.h Show resolved Hide resolved
src/ir/auth_logic/ast.h Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
markww
markww approved these changes Feb 9, 2022
View reviewed changes
Copy link
Contributor

@markww markww left a comment

Choose a reason for hiding this comment

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

LGTM other than commenting the btree_set.

Base automatically changed from lowering-to-datalog@aferr to main February 10, 2022 12:41
markww
markww reviewed Feb 10, 2022
View reviewed changes
src/ir/auth_logic/ast.h Outdated Show resolved Hide resolved
@aferr aferr mentioned this pull request Feb 10, 2022
Open
bgogul
bgogul reviewed Feb 10, 2022
View reviewed changes
Copy link
Collaborator

@bgogul bgogul left a comment

Choose a reason for hiding this comment

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

Some minor questions and comments.

src/ir/auth_logic/ast.h Outdated Show resolved Hide resolved
src/ir/auth_logic/lowering_ast_datalog.cc Show resolved Hide resolved
src/ir/auth_logic/ast.h Show resolved Hide resolved
src/ir/auth_logic/souffle_emitter.h Outdated Show resolved Hide resolved
src/ir/auth_logic/datalog_ir.h Show resolved Hide resolved
@aferr aferr requested a review from bgogul February 11, 2022 13:24
bgogul
bgogul reviewed Feb 11, 2022
View reviewed changes
src/ir/auth_logic/ast_equality_test.cc Outdated Show resolved Hide resolved
src/ir/auth_logic/ast_equality_test.cc Outdated Show resolved Hide resolved
@aferr aferr merged commit c94fcce into main Feb 11, 2022
@aferr aferr deleted the datalog_ir_emitter@aferr branch February 11, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgogul bgogul bgogul left review comments

@markww markww markww approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aferr @markww @bgogul