[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>,
----------------
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.


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

https://reviews.llvm.org/D54777