[PATCH] D92296: [AARCH64] Improve accumulator forwarding for Cortex-A57 model

[Usman Nadeem via Phabricator via llvm-commits]

mnadeem marked an inline comment as done.
mnadeem added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedA57.td:96
 def : SchedAlias<WriteFImm,  A57Write_3cyc_1V>;
-def : SchedAlias<WriteFMul,  A57Write_5cyc_1V>;
+// Replacing SchedAlias with WriteRes for advance lookup
+def : WriteRes<WriteFMul, [A57UnitV]> { let Latency = 5;}
----------------
dmgreen wrote:
> Can you update this comment to why this is different, not why it has _changed_ (which doesn't mean a lot once the code is in-tree.)
> 
> So something like "Use a WriteRes as opposed to SchedAlias for advance lookup"
Removed the comment because the warning on line 68 explains the change.
I felt that the comment was redundant. 


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedA57.td:419
+// ASIMD shift by register, basic, D-form
+def : InstRW<[A57Write_3cyc_1X_NonMul_Forward], (instregex "^[SU]SHL(v1i8|v1i16|v1i32|v1i64|v8i8|v4i16|v2i32|b|d|h|s)")>;
 
----------------
dmgreen wrote:
> What is a ^[SU]SHLv1i8 ?
> Same for all these others.
SSHL and USHL D-form. 

https://godbolt.org/z/a7jo7a 
The schedule was missing. These should only use the X Unit but seem to be using Units X/W



================
Comment at: llvm/lib/Target/AArch64/AArch64SchedA57WriteRes.td:54
 def A57Write_4cyc_1L  : SchedWriteRes<[A57UnitL]> { let Latency = 4;  }
+def A57Write_4cyc_1I  : SchedWriteRes<[A57UnitI]> { let Latency = 4;  }
 def A57Write_4cyc_1X  : SchedWriteRes<[A57UnitX]> { let Latency = 4;  }
----------------
dmgreen wrote:
> Where is this used?
Removed it. That was a mistake. 


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

https://reviews.llvm.org/D92296