This is an archive of the discontinued LLVM Phabricator instance.

[mips] Generate memory dependencies for byVal arguments
ClosedPublic

Authored by smaksimovic on Sep 6 2017, 7:10 AM.

Details

Summary

There were no memory dependencies made between stores generated when lowering formal arguments and loads generated when call lowering byVal arguments which made the Post-RA scheduler place a load before a matching store.

  • Make the fixed object stored to mutable so that the load instructions can have their memory dependencies added
  • Set the frame object as isAliased which clears the underlying objects vector in ScheduleDAGInstrs::buildSchedGraph(). This results in addition of all stores as dependenies for loads.

This problem appeared when passing a byVal parameter coupled with a fastcc function call.

Diff Detail

Event Timeline

smaksimovic created this revision.Sep 6 2017, 7:10 AM

Modified the test file to check only for the one offending SW/LW pair. The previous version was incorrect and it seemed difficult to match all the stores in arbitrary order followed by all loads also occurring arbitrarily.

sdardis added inline comments.Sep 14 2017, 3:56 AM
test/CodeGen/Mips/fastcc_byval.ll
2

-filetype=asm is unneeded here.

The previous version was found to cause problems elsewhere, the reason being different underlying objects which resulted in wrong ordering of two instructions.
This way we can keep the parameters in MachinePointerInfo() for the stores that are created in MipsTargetLowering::copyByValRegs().
Setting the frame object as isAliased clears the underlying objects vector in ScheduleDAGInstrs::buildSchedGraph() resulting in addition of all stores as dependenies for loads. The desired dependency is there now, the problem being the other ones which are not are there as well.

sdardis accepted this revision.Feb 16 2018, 7:22 AM

LGTM. Add a comment to test file, describing the purpose of the test.

lib/Target/Mips/MipsISelLowering.cpp
4105

Expand the comment here on why we're marking the object as mutable + isAliased.

This revision is now accepted and ready to land.Feb 16 2018, 7:22 AM
smaksimovic edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.