This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Retrieve the offset from load/store if it stores to stack slots
ClosedPublic

Authored by steven.zhang on Jul 22 2020, 4:23 AM.

Details

Summary

Scheduler will try to retrieve the offset and base addr to determine if two loads/stores are disjoint memory access. PowerPC failed to handle this for frame index which will bring extra memory dependency for loads/stores. See.

SU(15):   STW %7:gprc, 28, %fixed-stack.0 :: (store 4 into %fixed-stack.0 + 28)
  # preds left       : 1
  # succs left       : 2
  # rdefs left       : 0
  Latency            : 1
  Depth              : 1
  Height             : 7
  Predecessors:
    SU(0): Data Latency=1 Reg=%7
  Successors:
    SU(17): Ord  Latency=1 Memory
    SU(16): Ord  Latency=1 Memory
  Pressure Diff      : GPRC 1    SPILLTOVSRRC 1    SPILLTOVSRRC_with_VFRC 1    F4RC_with_SPILLTOVSRRC 1    VSSRC_with_SPILLTOVSRRC 1
  Single Issue       : false;
SU(16):   %8:gprc = LWZ 84, %fixed-stack.0 :: (dereferenceable load 4 from %ir.arrayidx_b + 4, align 8)
  # preds left       : 8
  # succs left       : 1
  # rdefs left       : 0
  Latency            : 1
  Depth              : 2
  Height             : 6
  Predecessors:
    SU(15): Ord  Latency=1 Memory   #<--- Extra dependency which is not needed.
    SU(14): Ord  Latency=1 Memory
    SU(13): Ord  Latency=1 Memory
    SU(12): Ord  Latency=1 Memory
    SU(11): Ord  Latency=1 Memory
    SU(10): Ord  Latency=1 Memory
    SU(9): Ord  Latency=1 Memory
    SU(8): Ord  Latency=1 Memory

Diff Detail

Event Timeline

steven.zhang created this revision.Jul 22 2020, 4:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2020, 4:23 AM
jsji accepted this revision as: jsji.Jul 28 2020, 2:16 PM

LGTM, but please update the testcases carefully, especially regarding DAG. -- Unnecessary DAG will hide bugs .

llvm/test/CodeGen/PowerPC/aix-cc-byval.ll
759

Nit: this shouldn't be DAG -- STW 20 should be before others because of following overlapped LBZ.

779–780

This should not be DAG as well. STD killed renamable $x5, 16 should be before STD killed renamable $x3,0 because of dependent LBZ8.

This revision is now accepted and ready to land.Jul 28 2020, 2:16 PM
steven.zhang added inline comments.Jul 29 2020, 10:22 PM
llvm/test/CodeGen/PowerPC/aix-cc-byval.ll
759

The DAG here means all the STW 20/0/4/8/12/16 are position independent. And then, we have a 32BIT as barrier, and then, another two position independent check which looks right from me.

779–780

STD 16 and STD 0 are independent store, so, we need the DAG here. And then, 64BIT-NEXT works as a barrier as it has dependent with above two stores. So, the DAG here looks right also.

jsji added inline comments.Jul 30 2020, 6:55 AM
llvm/test/CodeGen/PowerPC/aix-cc-byval.ll
759

Understand that they are independent, but if STW 20 is scheduled at the end, this should be a issue that we should be altered and looked into. Having DAG here will hide all such issues.

Update the test according to comments. But there are so many CHECK-DAG in this test that will hide some scheduling issue. I will leave it alone to have some other NFC patch to update the tests.

This revision was landed with ongoing or failed builds.Jul 31 2020, 12:13 AM
This revision was automatically updated to reflect the committed changes.