[PATCH] D117781: [flang] Update tco tool pipline and add translation to LLVM IR

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 11 00:29:06 PST 2022


clementval added a comment.

In D117781#3313556 <https://reviews.llvm.org/D117781#3313556>, @mehdi_amini wrote:

> In D117781#3260512 <https://reviews.llvm.org/D117781#3260512>, @clementval wrote:
>
>> In D117781#3260316 <https://reviews.llvm.org/D117781#3260316>, @mehdi_amini wrote:
>>
>>> LGTM, **assuming this tool is temporary and will get removed**. But I'd like to understand why the pipeline can't be all registered in `fir-opt`?
>>
>> I guess we can merge the two tools at some point. `tco` was there before `fir-opt` so nobody took the time to merge the functionality of one into the other yet.
>
> To be very clear in this revision because there seems to be some confusion in the chat:
>
> - You got a conditional LGTM here and you committed based on this condition.
> - You acknowledged that you're contributed a tool here that only exists because the functionalities haven't been merged yet to fir-opt, but that we can merge the tools in fir-opt (and so **delete tco** at this point) in the future.
>
> If this isn't very clear, please revert ASAP.



In D117781#3313556 <https://reviews.llvm.org/D117781#3313556>, @mehdi_amini wrote:

> In D117781#3260512 <https://reviews.llvm.org/D117781#3260512>, @clementval wrote:
>
>> In D117781#3260316 <https://reviews.llvm.org/D117781#3260316>, @mehdi_amini wrote:
>>
>>> LGTM, **assuming this tool is temporary and will get removed**. But I'd like to understand why the pipeline can't be all registered in `fir-opt`?
>>
>> I guess we can merge the two tools at some point. `tco` was there before `fir-opt` so nobody took the time to merge the functionality of one into the other yet.
>
> To be very clear in this revision because there seems to be some confusion in the chat:
>
> - You got a conditional LGTM here and you committed based on this condition.
> - You acknowledged that you're contributed a tool here that only exists because the functionalities haven't been merged yet to fir-opt, but that we can merge the tools in fir-opt (and so **delete tco** at this point) in the future.
>
> If this isn't very clear, please revert ASAP.

As I mentioned in the chat, we do not have the equivalent of `tco` at this point so it cannot be removed now.

Also to make things clearer the `tco` tool is the equivalent of `mlir-translate` in MLIR and `fir-opt` is the equivalent of `mlir-opt`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117781/new/

https://reviews.llvm.org/D117781



More information about the llvm-commits mailing list