[PATCH] D89876: [ARM][SchedModels] Convert IsLdrAm3RegOffPred to MCSchedPredicate

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMScheduleA57.td:31-33
+def IsLdrAm3RegOffPred : MCSchedPredicate<CheckIsRegOperand<2>>;
+def IsLdrAm3RegOffPredX2 : MCSchedPredicate<CheckIsRegOperand<3>>;
+def IsLdrAm3RegOffPredX3 : MCSchedPredicate<CheckIsRegOperand<4>>;
----------------
andreadb wrote:
> It seems to me that the original intent of `isAddrMode3OpImm` is to check if an operand is NOT the invalid register.
> 
> `isAddrMode3OpImm` also seems to require that the operand must be a register (otherwise the getReg() would have trigger an assertion failure - since isReg() would have returned false).
> 
> `CheckIsRegOperand(idx)` is simply testing if the operand at index `idx` is a register. However, it doesn't check the actual value of that register.
> 
> You should be able to use `CheckInvalidRegOperand`.
> Basically, the original implementation is equivalent to:
> `CheckNot<CheckInvalidRegOperand<index>>`.
> 
> That being said, in the original code, the return value from `isAddrMode3OpImm` is also negated (example):
> ```
> !TII->isAddrMode3OpImm(*MI, 1)
> ```
> 
> Assuming that my understanding of the code is correct, I guess that you should be able to just use a simple `CheckInvalidRegOperand` in your case.
> 
There is not operator in the original code `{!TII->isAddrMode3OpImm(*MI, 1)}` which I forgot about :(. I'll update the diff shortly



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

https://reviews.llvm.org/D89876