Page MenuHomePhabricator

[TargetLowering] Add MachineMemOperand::Flags to allowsMemoryAccess tests (PR42123)
ClosedPublic

Authored by RKSimon on Mon, Jun 10, 5:37 AM.

Details

Summary

As discussed on D62910, we need to check whether particular types of memory access are allowed, not just their alignment/address-space.

This NFC patch adds a MachineMemOperand::Flags argument to allowsMemoryAccess and allowsMisalignedMemoryAccesses, and wires up calls to pass the relevant flags to them.

If people are happy with this approach I can then update X86TargetLowering::allowsMisalignedMemoryAccesses to handle misaligned NT load/stores.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Mon, Jun 10, 5:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Jun 10, 5:37 AM
arsenm added inline comments.Mon, Jun 10, 6:04 AM
include/llvm/CodeGen/TargetLowering.h
1434 ↗(On Diff #203809)

Should probably add an overload that just takes a const MachineMemOperand&

RKSimon updated this revision to Diff 203811.Mon, Jun 10, 6:07 AM

Add missing flags to AMDGPUTargetLowering::performStoreCombine

RKSimon marked an inline comment as done.Mon, Jun 10, 6:10 AM
RKSimon added inline comments.
include/llvm/CodeGen/TargetLowering.h
1434 ↗(On Diff #203809)

Good idea!

RKSimon updated this revision to Diff 203815.Mon, Jun 10, 7:12 AM

Add TargetLoweringBase::allowsMemoryAccess helper wrapper - if people are OK with this I can add this wrapper (and the accompanying NFC refactor) as pre-commit.

arsenm added inline comments.Tue, Jun 11, 6:29 AM
include/llvm/CodeGen/TargetLowering.h
1430 ↗(On Diff #204046)

Tangentially related, but do we really need to pass the EVT here? I was trying to use this in GlobalISel recently, and this was an issue. The TTI wrapper already doesn't use a specific type

RKSimon marked an inline comment as done.Tue, Jun 11, 6:36 AM
RKSimon added inline comments.
include/llvm/CodeGen/TargetLowering.h
1430 ↗(On Diff #204046)

We might be able to get away with "unsigned DataSize", but I'm a little dubious - I'll know later on once I've got all the NT vector handling completed in x86.

LGTM.

include/llvm/CodeGen/TargetLowering.h
1430 ↗(On Diff #204046)

It's the data type that decides how the value is handled. In that sense it serves as the source of this kind of information, so it should be passed here, even if specific targets only need the size.

RKSimon updated this revision to Diff 204261.Wed, Jun 12, 4:35 AM

rebase after XCore cleanup - any more comments please?

kparzysz accepted this revision.Wed, Jun 12, 8:16 AM
This revision is now accepted and ready to land.Wed, Jun 12, 8:16 AM
This revision was automatically updated to reflect the committed changes.