This is an archive of the discontinued LLVM Phabricator instance.

[DAG] CombineConsecutiveLoads - replace getABITypeAlign with allowsMemoryAccess (PR45116)
ClosedPublic

Authored by RKSimon on Aug 18 2021, 10:19 AM.

Details

Summary

One of the cases identified in PR45116 - we don't need to limit load combines (in this case for ISD::BUILD_PAIR) to ABI alignment, we can use allowsMemoryAccess - which tests using getABITypeAlign, but also checks if a target permits misaligned memory loads by checking allowsMisalignedMemoryAccesses as a fallback.

This helps in particular for 32-bit X86 cases loading 64-bit size data, reducing codegen diffs vs x86_64.

Diff Detail

Event Timeline

RKSimon created this revision.Aug 18 2021, 10:19 AM
RKSimon requested review of this revision.Aug 18 2021, 10:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 18 2021, 10:19 AM
pengfei added inline comments.Aug 23 2021, 1:54 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
12573

Is the type possible a vector? Is there performance penalty when we turn it into unaligned?

RKSimon added inline comments.Aug 23 2021, 2:08 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
12573

allowsMemoryAccess does have an optional fast check that we could use, that allows targets to specify if the unaligned load is slower or not - I'll try adding it.

RKSimon updated this revision to Diff 368052.Aug 23 2021, 2:55 AM

Limit fold to fast memory access

RKSimon added inline comments.Aug 23 2021, 9:08 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
12573

I've added the fast check to ensure we only perform the unaligned load if its not slow - X86TargetLowering::allowsMisalignedMemoryAccesses checks for slow unaligned memory access to avoid perf issues with vectors.

pengfei accepted this revision.Aug 23 2021, 11:39 PM

LGTM.

This revision is now accepted and ready to land.Aug 23 2021, 11:39 PM
This revision was landed with ongoing or failed builds.Aug 24 2021, 4:37 AM
This revision was automatically updated to reflect the committed changes.