[PATCH] D54777: [AArch64] Refactor the scheduling predicates (1/3) (NFC)

[Evandro Menezes via Phabricator via llvm-commits]

evandro marked 4 inline comments as done.
evandro added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredicates.td:61-62
+// with an extended or scaled register.
+def ScaledIdxBody : CheckAll<[CheckAny<[IsLoadRegOffsetPred,
+                                        IsStoreRegOffsetPred]>,
+                              CheckAny<[CheckNot<CheckMemExtLSL>,
----------------
andreadb wrote:
> andreadb wrote:
> > evandro wrote:
> > > andreadb wrote:
> > > > If `IsLoadRegOffsetPred` and `IsStoreRegOffsetPred` are only used by the definition of `ScaledIdxBody`, then you should simply use a `MCOpcodeSwitchStatement`.
> > > > 
> > > > I had a look at your three NFC patches; this is the only place where you use `IsLoadRegOffsetPred` and `IsStoreRegOffsetPred`. 
> > > > 
> > > > I know that in one of your previous comments you mentioned a prolem with `MCOpcodeSwitchStatement`. I still don't understand what the problem is. When I tried to rewrite your patch using `MCOpcodeSwitchStatement` everything worked fine for me...
> > > > I wonder if the issue is caused by the way you defined `ScaledIdxPred`. That definition should be changed to reference `ScaledIdxFn` directly (See my comment below).
> > > Yes, I intend to reuse both `IsLoadRegOffsetPred` and `IsStoreRegOffsetPred` in a future patch.  This is the reason; nothing wrong with using a more elegant switch statement in the emitted code.
> > I think that you can use a `!listconcat` operator to concatenate the opcodes from those two predicates.
> > 
> > Something like this (I didn't verify that it works though...):
> > 
> > ```
> > def ScaledIdxFn : TIIPredicate<"isScaledAddr",
> >   MCOpcodeSwitchStatement<[
> >     MCOpcodeSwitchCase<
> >       !listconcat(IsLoadRegOffsetPred.ValidOpcodes, IsStoreRegOffsetPred.ValidOpcodes),
> >       MCReturnStatement<CheckAny<[
> >         CheckNot<CheckExtendType>,
> >         CheckFoldedShift ]>>
> >     >], MCReturnStatement<FalsePred>
> >   >
> > >;
> > ```
> > 
> > I think it should work. It would let you use the switch statement.
> By the way. If `!listconcat` works for you (and you are happy with it), then you can use it to improve the other two NFC patches too.
It seems to work.  Will do.


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

https://reviews.llvm.org/D54777