[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