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

[WIP] LLVM rebasing efforts #359

Merged
merged 2 commits into from
Oct 6, 2023
Merged

[WIP] LLVM rebasing efforts #359

merged 2 commits into from
Oct 6, 2023

Conversation

ivanradanov
Copy link
Collaborator

@ivanradanov ivanradanov commented Sep 7, 2023

(This is on top of #355)

WIP PR for rebasing to latest llvm-project.

As llvm deprecated typed pointers we would need to eventually remove all usage of them in Polygeist as well if my understanding is correct.

Another big part that is affected by the removal of typed llvm pointers is the AST->MLIR frontend (cgeist).
The LLVM dialect code-gen there is mostly unused as it was for ABI compatibility issues we have mostly solved.

So I would say we should remove all usage of the LLVM dialect (except the struct and array types) in the frontend while doing the rebase to make our lives easier. This might require some new additions to the polygeist dialect.

  • Fix backend LLVM dialect codegen
  • (Optional) Add Polygeist struct/class type to use instead of LLVM::Struct
    • Remove LLVM dialect codegen in frontend
    • Variadic functions without LLVMFuncOp?
  • I did not manage to figure out how to add dependencies of passes on dialect extension (specifically parallel-lower depends on the inliner interface extension of func), fix?
  • polygeist-opt tests
  • cgeist tests

@@ -5778,6 +5778,10 @@ LogicalResult fixupGetFunc(LLVM::CallOp op, OpBuilder &rewriter,

Value pval = op.getOperand(0);

// We can't get type info from opaque pointer which we now use TODO what do we
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@wsmoses

Can you take a look at what we should be doing here instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Most of the remaining failing tests are related to this

Copy link
Member

Choose a reason for hiding this comment

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

in LLVM itself there is a getFunctionType from the CallInst.

we should use the MLIR-equivalent of that here to get FT

Copy link
Member

Choose a reason for hiding this comment

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

LLVM: https://llvm.org/doxygen/InstrTypes_8h_source.html#l01270

MLIR: https://github.com/llvm/llvm-project/blob/b300f8c89829f920102e6ec5acc7d1b134bcff75/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td#L628

I don't immediately see anything upstream MLIR that does the LLVM similar. @ftynse if you know offhand

If not we may need to add support to upstream MLIR for

Copy link
Member

Choose a reason for hiding this comment

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

I think the assumption so far was that functions are non-variadic so the function type can be trivially reconstructed from the lists of operands and results of the call op. For variadic functions, there's a FIXME just above: https://github.com/llvm/llvm-project/blob/b300f8c89829f920102e6ec5acc7d1b134bcff75/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td#L584-L586

@ivanradanov
Copy link
Collaborator Author

ivanradanov commented Sep 28, 2023

@wsmoses I think this is now ready?
I have XFAIL'ed two of the polygeist-opt tests because they need more substantial changes - namely fixing LLVM Opaque Pointer mem2reg and fixing a bug in the AffineBufferElimination optimization (currently disabled)

This also contains the changes in #355, (If they look good) I will squash in two different commits I think, one for the previous pr, and one for this.

@ivanradanov ivanradanov marked this pull request as ready for review September 28, 2023 07:31
* More flexible specification of coarsening factors

* Fix device-side get_global ops

* For PGO Output the accumulated runtime of kernels, not individual

* Allow for granular blcok coarsening factors

* Various GPU bug fixes

* Remove unneeded kernels from the gpu binary

* Collect kernel statistics

* Add polygeist::UndefOp for cases when the type is not LLVM::
@ivanradanov ivanradanov merged commit 50d4f21 into main Oct 6, 2023
3 checks passed
@ivanradanov ivanradanov deleted the ivan_llvm_bump branch October 6, 2023 07:32
@ivanradanov ivanradanov mentioned this pull request Oct 6, 2023
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.

3 participants