tensorflow / mlir

"Multi-Level Intermediate Representation" Compiler Infrastructure
1.73k stars 257 forks source link

[LLVM] Implement missing LLVM IR instructions #276

Open ftynse opened 4 years ago

ftynse commented 4 years ago

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.

uenoku commented 4 years ago

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

ftynse commented 4 years ago

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.

uenoku commented 4 years ago

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

bondhugula commented 4 years ago

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

ftynse commented 4 years ago

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.

bondhugula commented 4 years ago

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.

antiagainst commented 4 years ago

+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.

ftynse commented 4 years ago

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.

shraiysh commented 4 years ago

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

marbre commented 4 years ago

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

ftynse commented 4 years ago

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 notifications@github.com 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