This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Fix usage of Align constructing MachineMemOperands
ClosedPublic

Authored by efriedma on Apr 7 2020, 3:42 PM.

Details

Summary

The "Align" passed into getMachineMemOperand etc. is the alignment of the MachinePointerInfo, not the alignment of the memory operation. (getAlign() on a MachineMemOperand automatically reduces the alignment to account for this.)

We were passing on wrong (overconservative) alignment in a bunch of places. Fix a bunch of these, mostly in legalization. And while I'm here, switch to the new Align APIs.

The test changes are all scheduling changes: the biggest effect of preserving large alignments is that it improves alias analysis, so the scheduler has more freedom.

(I was originally just trying to do a minor cleanup in SelectionDAGBuilder, but I accidentally went deeper down the rabbit hole.)

Diff Detail

Event Timeline

efriedma created this revision.Apr 7 2020, 3:42 PM
efriedma updated this revision to Diff 256886.Apr 12 2020, 3:06 PM
efriedma edited the summary of this revision. (Show Details)

Rebased, fixed test failures, updated commit message.

Do you intend to split the commits at all?

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
437

Incidently, should we tag MemSDNode::getAlignment() as LLVM_ATTRIBUTE_DEPRECATED now? We already have this for MemSDNode::getOriginalAlignment().

efriedma marked an inline comment as done.Apr 13 2020, 10:22 AM

I wasn't really planning to split this, but I can see why you'd want me to. I can at least split off the MachineMemOperand change from the rest.

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
437

I don't think it makes sense to mark things deprecated in CodeGen; if they have no uses, we can kill them off. If they have uses, we can't mark them deprecated because it'll break -Werror builds.

Split off the MachineMemOperand change. Please let me know if you want me to split this further.

efriedma updated this revision to Diff 257540.Apr 14 2020, 3:49 PM
efriedma edited the summary of this revision. (Show Details)

Rebased

RKSimon accepted this revision.Apr 15 2020, 5:33 AM

LGTM - cheers

This revision is now accepted and ready to land.Apr 15 2020, 5:33 AM
lkail added a subscriber: lkail.Apr 15 2020, 7:05 AM
This revision was automatically updated to reflect the committed changes.