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

Init commit for sql dialect #319

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Init commit for sql dialect #319

wants to merge 15 commits into from

Conversation

carlguo866
Copy link
Collaborator

No description provided.

@carlguo866 carlguo866 requested review from wsmoses and ftynse January 31, 2023 22:53
lib/sql/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -0,0 +1,27 @@
//===- PolygeistDialect.cpp - Polygeist dialect ---------------*- C++ -*-===//
Copy link
Member

Choose a reason for hiding this comment

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

rename to sql?

@@ -0,0 +1,40 @@
//===- PolygeistOps.cpp - BFV dialect ops ---------------*- C++ -*-===//
//
Copy link
Member

Choose a reason for hiding this comment

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

rename to sql?

@carlguo866 carlguo866 requested a review from wsmoses February 4, 2023 16:36
Comment on lines +6 to +8
%q = "sql.select"() {column = ["data"], table = "mytable"} : () -> index
%h = "sql.execute"(%q) : (index) -> index
%res = "sql.get_result"(%h, %c0) {column = "data"} : (index, index) -> i32
Copy link
Member

Choose a reason for hiding this comment

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

For this to work, you need to call allowUnknonwnOperations in the dialect initializer.

Copy link
Member

Choose a reason for hiding this comment

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

Is that needed? These operations are registered in the dialect?

@carlguo866
Copy link
Collaborator Author

Including add_mlir_doc(SQLDialect -gen-dialect-doc SQLDialect SQL/) and add_mlir_doc(SQLOps -gen-op-doc SQLOps SQL/) give the following error:

CMake Error at /home/carl/Polygeist/llvm-project/mlir/cmake/modules/AddMLIR.cmake:200 (add_custom_target):
add_custom_target cannot create target "-gen-dialect-docDocGen" because
another target with the same name already exists. The existing target is a
custom target created in source directory
"/home/carl/Polygeist/include/polygeist". See documentation for policy
CMP0002 for more details.
Call Stack (most recent call first):
/home/carl/Polygeist/include/sql/CMakeLists.txt:2 (add_mlir_doc)

CMake Error at /home/carl/Polygeist/llvm-project/mlir/cmake/modules/AddMLIR.cmake:200 (add_custom_target):
add_custom_target cannot create target "-gen-op-docDocGen" because another
target with the same name already exists. The existing target is a custom
target created in source directory
"/home/carl/Polygeist/include/polygeist". See documentation for policy
CMP0002 for more details.
Call Stack (most recent call first):
/home/carl/Polygeist/include/sql/CMakeLists.txt:3 (add_mlir_doc)

CMakeLists.txt Outdated
@@ -1,110 +0,0 @@
cmake_minimum_required(VERSION 3.10)
Copy link
Member

Choose a reason for hiding this comment

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

Can we leave this file

@@ -73,7 +73,9 @@
#include <fstream>

#include "polygeist/Dialect.h"
#include "sql/SQLDialect.h"
Copy link
Member

Choose a reason for hiding this comment

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

have these inside of #ifdef ENABLE_SQL

Copy link
Member

Choose a reason for hiding this comment

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

you're going to need to in cmake, if enable_sql then add an enable sql in c

Copy link
Collaborator

Choose a reason for hiding this comment

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

(see POLYGEIST_ENABLE_CUDA for an example)

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.

4 participants