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

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb 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:
> 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.


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

https://reviews.llvm.org/D54777