[PATCH] D55375: [AArch64] Refactor the scheduling predicates

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredicates.td:354
+                                              [CheckZeroOperand<1>,
+                                               CheckZeroOperand<2>]>>>],
+                                       MCReturnStatement<FalsePred>>>;
----------------
evandro wrote:
> andreadb wrote:
> > I am not familiar with aarch64. So, apologies if what I am about to ask is silly...
> > 
> > The comment at line 347 suggest that you want to match a `MOVI Vd, #0, LSL #0`.
> > 
> > My question is: is the check for operand #2 correct? Is that check equivalent to checking for the presence of a LSL #0? If so, then fine.
> > 
> > This is the tablegen'd definition:
> > 
> > ```
> > bool AArch64InstrInfo::isZeroFPIdiom(const MachineInstr &MI) {
> >   switch(MI.getOpcode()) {
> >   case AArch64::MOVIv8b_ns:
> >   case AArch64::MOVIv16b_ns:
> >   case AArch64::MOVID:
> >   case AArch64::MOVIv2d_ns:
> >     return MI.getOperand(1).getImm() == 0;
> >   case AArch64::MOVIv4i16:
> >   case AArch64::MOVIv8i16:
> >   case AArch64::MOVIv2i32:
> >   case AArch64::MOVIv4i32:
> >     return (
> >       MI.getOperand(1).getImm() == 0
> >       && MI.getOperand(2).getImm() == 0
> >     );
> >   default:
> >     return false;
> >   } // end of switch-stmt
> > }
> > ```
> Correct.  The 3rd operand is an immediate bit field  comprising 2 bits to encode the shift (with LSL being the only possible one) and 6 bits to encode the shift amount (with the only possible values being a multiple of 8).  Checking it for zero guarantees LSL by 0. 
Thanks for the explanation. LGTM 


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

https://reviews.llvm.org/D55375