Page MenuHomePhabricator

[AArch64] Refactor the scheduling predicates

Authored by evandro on Dec 6 2018, 8:40 AM.



Refactor the scheduling predicates based on MCInstPredicate. Augment the number of helper predicates used by processor specific predicates.

Diff Detail


Event Timeline

evandro created this revision.Dec 6 2018, 8:40 AM
andreadb added inline comments.
17–27 ↗(On Diff #176986)

Needed by D55345 too.
Without that definition, patch D55345 will not build because CheckExtUXTW is not defined (see

29–33 ↗(On Diff #176986)

This is needed by D55345 too.

64–65 ↗(On Diff #176986)

Needed by D55345.

100 ↗(On Diff #176986)


135 ↗(On Diff #176986)

This definition is also needed by your other patch D55345 in

137 ↗(On Diff #176986)

It is required by D55345 in

Note that I made this patch a predecessor for D55345.

Note that I made this patch a predecessor for D55345.

Thanks for clarifying it. I missed the phabricator message where it says that this is a parent patch...

I am going to review it now.

evandro updated this revision to Diff 177008.Dec 6 2018, 10:42 AM
andreadb accepted this revision.Dec 6 2018, 12:43 PM

I only have one question. Otherwise, the new predicates LGTM (their logic seems to match what you wrote in the code comments).

354 ↗(On Diff #177008)

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
    return false;
  } // end of switch-stmt
This revision is now accepted and ready to land.Dec 6 2018, 12:43 PM
evandro marked 2 inline comments as done.Dec 6 2018, 3:48 PM
evandro added inline comments.
354 ↗(On Diff #177008)

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.

andreadb added inline comments.Dec 6 2018, 3:52 PM
354 ↗(On Diff #177008)

Thanks for the explanation. LGTM

This revision was automatically updated to reflect the committed changes.
evandro marked an inline comment as done.