This is an archive of the discontinued LLVM Phabricator instance.

[x86] Propagate memory operands during call frame optimization
ClosedPublic

Authored by Kayjukh on May 16 2020, 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.May 16 2020, 5:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2020, 5:04 AM

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

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

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

571

This also has a store to stack

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

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?

571

Same comment as above.

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

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

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

Also propagate store memory operands during call frame optimization.

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

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

Kayjukh marked an inline comment as done.May 16 2020, 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.May 18 2020, 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.May 20 2020, 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)May 20 2020, 7:06 AM
This revision is now accepted and ready to land.May 28 2020, 8:39 AM
This revision was automatically updated to reflect the committed changes.