Page MenuHomePhabricator

[x86] Propagate memory operands during call frame optimization
ClosedPublic

Authored by Kayjukh on Sat, May 16, 5:04 AM.

Details

Summary

Propagate memory operands when folding load instructions into instructions that directly operate on memory.

The original revision has been split. See D80140 for the other part of the changes.

Diff Detail

Event Timeline

Kayjukh created this revision.Sat, May 16, 5:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptSat, May 16, 5:04 AM

The change to X86ISelDAGToDAG.cpp looks fine. I have questions about the other change.

llvm/lib/Target/X86/X86CallFrameOptimization.cpp
566

This captures the memoperand for the load of the source, but what about the store to the stack?

572

This also has a store to stack

Kayjukh marked 2 inline comments as done.Sat, May 16, 12:02 PM
Kayjukh added inline comments.
llvm/lib/Target/X86/X86CallFrameOptimization.cpp
566

I'm not really sure how to model this. As far as I can tell, we don't know the location to which the value gets stored on the stack. Would you add a memory operand with the MachinePointerInfo returned by MachinePointerInfo::getUnknownStack?
Or am I missing something?

572

Same comment as above.

craig.topper added inline comments.
llvm/lib/Target/X86/X86CallFrameOptimization.cpp
566

Does the Store node that we're replacing have a mem operand?

Kayjukh updated this revision to Diff 264451.Sat, May 16, 12:55 PM

Also propagate store memory operands during call frame optimization.

Kayjukh marked 4 inline comments as done.Sat, May 16, 12:56 PM
Kayjukh added inline comments.
llvm/lib/Target/X86/X86CallFrameOptimization.cpp
566

Yes, I overlooked the store. Should be good now!

Kayjukh marked an inline comment as done.Sat, May 16, 1:02 PM

Can there be test coverage for this?

Can there be test coverage for this?

For the X86ISelDAGToDAG.cpp change it should be possible to do -stop-after=finalize-isel and inspect the MIR output.

For the out we should be able to have .mir input that just runs the call frame optimization pass.

We should split these two changes into two reviews.

Kayjukh updated this revision to Diff 264663.Mon, May 18, 9:41 AM
Kayjukh retitled this revision from [x86] Propagate memory operands during call frame optimization & ISel DAG postprocessing to [x86] Propagate memory operands during call frame optimization.

Split the call frame optimization patch from the ISel DAG postprocessing patch.

Code looks reasonable. It would be nice to have an MIR testcase showing the memoperands after this pass runs, even if there isn't any change to the final asm without additional work.

Kayjukh updated this revision to Diff 265231.Wed, May 20, 7:05 AM

Also propagate memory operands when folding non-MOV instructions. Add a test case.

Kayjukh edited the summary of this revision. (Show Details)Wed, May 20, 7:06 AM
This revision is now accepted and ready to land.Thu, May 28, 8:39 AM
This revision was automatically updated to reflect the committed changes.