This is an archive of the discontinued LLVM Phabricator instance.

[X86][DAG] Switch X86 Target to post-legalized store merge
ClosedPublic

Authored by niravd on Jun 23 2017, 9:20 AM.

Details

Summary

Move store merge to happen after intrinsic lowering to allow lowered
stores to be merged.

Some regressions due in MergeConsecutiveStores to missing
insert_subvector that are addressed in follow up patch.

Diff Detail

Repository
rL LLVM

Event Timeline

niravd created this revision.Jun 23 2017, 9:20 AM
niravd planned changes to this revision.Jun 23 2017, 9:45 AM
niravd updated this revision to Diff 103762.Jun 23 2017, 1:09 PM

Rebase on improved bitcast store

niravd edited the summary of this revision. (Show Details)Jun 23 2017, 1:10 PM
RKSimon added inline comments.Jun 24 2017, 6:16 AM
test/CodeGen/X86/oddshuffles.ll
86 ↗(On Diff #103762)

This is scaring me - what is the reasoning behind permitting the store of a <4 x i32> and not just the <3 x i32> ?

niravd updated this revision to Diff 104262.Jun 27 2017, 1:53 PM

Rebase given corrected D34569 patch which was failing to check candidate store sizes in some merging cases.

niravd added inline comments.Jun 27 2017, 1:56 PM
test/CodeGen/X86/oddshuffles.ll
86 ↗(On Diff #103762)

This is due to a mistake in a parent patch which dropped a type size check. It should be fixed now.

niravd edited the summary of this revision. (Show Details)Jun 27 2017, 6:32 PM
niravd updated this revision to Diff 108511.Jul 27 2017, 11:45 AM

Rebase and general update. No real change.

RKSimon added inline comments.Jul 31 2017, 5:50 AM
lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
61 ↗(On Diff #108511)

One use and fits on the line, so we can just do:

SDValue Base = DAG.getTargetLoweringInfo().unwrapAddress(Ptr);
test/CodeGen/X86/bitcast-i256.ll
25 ↗(On Diff #108511)

Get rid of this - did the script add it?

niravd updated this revision to Diff 108915.Jul 31 2017, 6:43 AM
niravd marked 2 inline comments as done.

Address comments.

RKSimon added inline comments.Aug 1 2017, 6:21 AM
test/CodeGen/X86/bigstructret.ll
1 ↗(On Diff #108915)

I've regenerated this test file, so it needs rebasing.

test/CodeGen/X86/extract-store.ll
528 ↗(On Diff #108915)

Any idea why this still doesn't merge? Same for @extract_f128_1 as well.

niravd updated this revision to Diff 109115.Aug 1 2017, 7:41 AM

Update test.

niravd marked an inline comment as done.Aug 1 2017, 7:41 AM
niravd added inline comments.
test/CodeGen/X86/extract-store.ll
528 ↗(On Diff #108915)

At merge time the component stores have an alignment of 1 and TLI.allowsMisalignedMemoryAccess believe this is legal but slow because Subtarget.isUnalignedMem16Slow() is set so we don't bother merging.

RKSimon accepted this revision.Aug 1 2017, 8:55 AM

LGTM

This revision is now accepted and ready to land.Aug 1 2017, 8:55 AM

@niravd Anything stopping this going in?

This revision was automatically updated to reflect the committed changes.