[PATCH] D69998: [MacroFusion] Create the missing artificial edges if there are more than 2 SU fused.

Qing Shan Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 10 18:33:29 PST 2019


steven.zhang added a comment.

In D69998#1739692 <https://reviews.llvm.org/D69998#1739692>, @jsji wrote:

> My understanding is that you are doing two fixes in this patch:
>
> 1. Extend API of `shouldScheduleAdjacent` to `avoid fusing an instruction more than once along the same dependency chain`.
> 2. Extend `fuseInstructionPair` to add artificial data dependencies for chained-fusion
>
> To me, yes, we should do #1 first, as https://reviews.llvm.org/D36704 was already trying to do so.


Yes, but still have problems.

> #2 might not be necessary, as you mentioned, all existing targets only support `back-to-back` fusion.
>  If we want to extend it for `chained-fusion` (3 or more in same dependency chain),
>  then I believe we need more changes than adding data dependencies.
>  Also need additional tests for some target that support it (maybe RISC-V?)

The current implementation has already been implemented to support more than 2 SU's fusion, However, it misses to create some dependency edges. The patch is trying to fix the bug of the macro fusion infrastructure. From my understanding,
adding these missing edges is enough. I didn't see llvm support the macro fusion for RISC-V target. AMDGPU supports more than 2 SU's cluster for load.  @arsenm Would you please help me confirm if AMDGPU target supports more than 2 SU's fusion ?


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

https://reviews.llvm.org/D69998





More information about the llvm-commits mailing list