mlir icon indicating copy to clipboard operation
mlir copied to clipboard

[LLVM] Implement missing LLVM IR instructions

Open ftynse opened this issue 6 years ago • 11 comments

The following LLVM IR instructions are currently not modeled by the LLVM dialect. Each of this is a great starter bug to implement a new Op in MLIR.

  • [ ] switch (work in progress)
  • [ ] indirectbr
  • [ ] invoke
  • [ ] callbr
  • [ ] resume
  • [ ] catchswitch
  • [ ] catchret
  • [ ] cleanupret
  • [ ] fence
  • [ ] cmpxchg
  • [ ] atomicrmw
  • [ ] freeze
  • [ ] va_arg
  • [ ] landingpad
  • [ ] catchpad
  • [ ] cleanuppad

ftynse avatar Nov 26 '19 20:11 ftynse

Hi, I'm interested in this. May I work on some of these instructions?

uenoku avatar Nov 27 '19 17:11 uenoku

You are very welcome to! Consider mentioning here which ones do you work on, in case somebody else wants to take the others.

include/Dialect/LLVMIR/LLVMOps.td has definitions of existing ops and should be fine as a template for more ops. Feel free to ping me as on the pull request when ready. Don't hesitate to reach out to the mailing list if you have questions.

ftynse avatar Nov 27 '19 18:11 ftynse

Thanks. Then, I'd like to work on switch instruction first.

uenoku avatar Nov 27 '19 18:11 uenoku

The following LLVM IR instructions are currently not modeled by the LLVM dialect. Each of this is a great starter bug to implement a new Op in MLIR.

  • [ ] switch
  • [ ] indirectbr
  • [ ] invoke
  • [ ] callbr
  • [ ] resume
  • [ ] catchswitch
  • [ ] catchret
  • [ ] cleanupret
  • [ ] fence
  • [ ] cmpxchg
  • [ ] atomicrmw
  • [ ] freeze
  • [ ] va_arg
  • [ ] landingpad
  • [ ] catchpad
  • [ ] cleanuppad

It would be good to add some of the useful LLVM intrinsics to this list. llvm.prefetch is already done by this PR: https://github.com/tensorflow/mlir/pull/225

bondhugula avatar Dec 02 '19 11:12 bondhugula

Define "useful". If we count extensions like NVVM, there are literally thousands of intrinsics...

Fortunately, they are defined in Tablegen files unlike instructions, so my general thinking is that we should auto-generate all of them. I have a prototype dusting in the corner if somebody is willing to pick it up.

ftynse avatar Dec 02 '19 12:12 ftynse

Define "useful". If we count extensions like NVVM, there are literally thousands of intrinsics...

It's a bit hard to define, but what I meant was "more commonly used" or "of imminent use" based on what we hear on the MLIR mailing list / design meetings. It did appear to me that a number of things in the list you have above aren't really of immediate use. Eg: landingpad, freeze, atomic memory ordering - is there a reason to have these before things like llvm.sqrt, llvm.powi, llvm.memcpy? The question of auto-generation is separate - they still have to be added to the TableGen file, right? Or did you mean auto-generating the .td file descriptions as well?

Fortunately, they are defined in Tablegen files unlike instructions, so my general thinking is that we should auto-generate all of them. I have a prototype dusting in the corner if somebody is willing to pick it up.

bondhugula avatar Dec 02 '19 12:12 bondhugula

+1 to autogen the .td file. FWIW, that's the approach we are using for the SPIR-V dialect. We have a gen_spirv_dialect.py script that downloads both the HTML and JSON grammar and updates SPIRV*.td files automatically. It is very handy for defining new ops and makes it easy to stay up to date with the spec too.

antiagainst avatar Dec 02 '19 12:12 antiagainst

It's a bit hard to define, but what I meant was "more commonly used" or "of imminent use" based on what we hear on the MLIR mailing list / design meetings.

I don't have data on what are the most commonly used ones, and it may depend on use cases :) Therefore, I wouldn't make a judgement call about usefulness here. If there are imminent users, they are free to add intrinsics on per-need basis. This has been the case for the LLVM dialect in the past, but as we the project grows and moves to LLVM, we aim to be feature-complete in modeling core LLVM IR.

The question of auto-generation is separate - they still have to be added to the TableGen file, right? Or did you mean auto-generating the .td file descriptions as well?

There is this file https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/IR/Intrinsics.td that defines (or transitively includes) intrinsics. Whether we generate an ODS file from it or use it directly to generate MLIR Op definitions in C++ is up for discussion.

ftynse avatar Dec 02 '19 13:12 ftynse

Hi, I would like to take up indirectbr as my first issue. Will take it up in the llvm monorepo.

shraiysh avatar Dec 20 '19 05:12 shraiysh

Is this issue still up to date? So far it was at least not transferred to LLVM bug tracker, as mentioned in docs.

marbre avatar Jan 02 '20 11:01 marbre

We have not migrated issues (yet).

AFAIK, there's ongoing work on switch, invoke and landingpad.

On Thu, Jan 2, 2020 at 12:42 PM Marius Brehler [email protected] wrote:

Is this issue still up to date? So far it was at least not transferred to LLVM bug tracker https://bugs.llvm.org/buglist.cgi?keywords=beginner%2C%20&keywords_type=allwords&list_id=176893&product=MLIR&query_format=advanced&resolution=---, as mentioned in docs https://mlir.llvm.org/getting_started/Contributing/ .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tensorflow/mlir/issues/276?email_source=notifications&email_token=AALRG237WRQDC7A46WIUIHLQ3XHK7A5CNFSM4JR5FSM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH6FIVY#issuecomment-570184791, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALRG2ZGTFGWQF6RSJRFUP3Q3XHK7ANCNFSM4JR5FSMQ .

-- Alex

ftynse avatar Jan 02 '20 12:01 ftynse