[PATCH] D90150: [ARM][SchedModels] Convert IsLdstsoScaledNotOptimalPred to MCSchedPredicate

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 26 07:08:41 PDT 2020


andreadb added a comment.

The new predicate looks good. I only have a couple of nits (see below).



================
Comment at: llvm/lib/Target/ARM/ARMScheduleA57.td:45-51
+class ScaledRegNotPlusLsl2<int n> : CheckNot<
+                                      CheckAny<[
+                                        CheckAM2NoShift<n>,
+                                        CheckAll<[
+                                          CheckAM2OpAdd<n>,
+                                          CheckAM2ShiftLSL<n>,
+                                          CheckAM2Offset<n, 2>]>]>>;
----------------
I wonder if this could be formatted in a more readable way.
Maybe the CheckNot could go to a new line.
The last line might be split into multiple lines maybe. The `>]>]>>` sequence is not very nice.


================
Comment at: llvm/lib/Target/ARM/ARMScheduleA57.td:53-55
+def IsLdstsoScaledNotOptimalPredX0 : MCSchedPredicate<ScaledRegNotPlusLsl2<2>>;
+def IsLdstsoScaledNotOptimalPred : MCSchedPredicate<ScaledRegNotPlusLsl2<3>>;
+def IsLdstsoScaledNotOptimalPredX2 : MCSchedPredicate<ScaledRegNotPlusLsl2<4>>;
----------------
What about the original `isLdstScaledRegNotPlusLsl2` ?
Is it still used by something else other than the scheduling model?
If not, then it should be removed.


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

https://reviews.llvm.org/D90150



More information about the llvm-commits mailing list