This is an archive of the discontinued LLVM Phabricator instance.

[X86][AMDGPU][DAGCombiner] Move call to allowsMemoryAccess into isLoadBitCastBeneficial/isStoreBitCastBeneficial to allow X86 to bypass it
ClosedPublic

Authored by craig.topper on Jul 6 2019, 11:45 PM.

Details

Summary

Basically the problem is that X86 doesn't set the Fast flag from
allowsMemoryAccess on certain CPUs due to slow unaligned memory
subtarget features. This prevents bitcasts from being folded into
loads and stores. But all vector loads and stores of the same width
are the same cost on X86.

This patch merges the allowsMemoryAccess call into isLoadBitCastBeneficial to allow X86 to skip it.

Event Timeline

craig.topper created this revision.Jul 6 2019, 11:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2019, 11:45 PM
arsenm added inline comments.Jul 7 2019, 3:17 PM
llvm/include/llvm/CodeGen/TargetLowering.h
405

It isn't clear to me what this does the name, so needs a better name? Also needs a documentation comment for it

craig.topper marked an inline comment as done.Jul 7 2019, 3:45 PM
craig.topper added inline comments.
llvm/include/llvm/CodeGen/TargetLowering.h
405

Do you have a better suggestion for a name? 'CallAllowsMemoryAccess'?

Or maybe I should just make isLoadBitCastBeneficial call the allowsMemoryAccess instead of DAGCombiner? Then X86 can control the behavior directly?

arsenm added inline comments.Jul 8 2019, 9:38 AM
llvm/include/llvm/CodeGen/TargetLowering.h
405

Calling allowsMemoryAccess directly would probably be better

Move the allowsMemoryAccess call into isLoadBitCastBeneficial

craig.topper retitled this revision from [X86][AMDGPU] Add an out parameter to isLoadBitCastBeneficial/isStoreBitCastBeneficial to indicate we shouldn't both checking the alignment. to [X86][AMDGPU][DAGCombiner] Move call to allowsMemoryAccess into isLoadBitCastBeneficial/isStoreBitCastBeneficial to allow X86 to bypass it.Jul 8 2019, 10:06 PM
craig.topper edited the summary of this revision. (Show Details)
arsenm accepted this revision.Jul 9 2019, 7:19 AM
This revision is now accepted and ready to land.Jul 9 2019, 7:19 AM
This revision was automatically updated to reflect the committed changes.