This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Handle odd splits in fewerElementsVector for load/store
ClosedPublic

Authored by arsenm on Jan 24 2019, 9:41 AM.

Diff Detail

Event Timeline

arsenm created this revision.Jan 24 2019, 9:41 AM
arsenm updated this revision to Diff 183778.Jan 27 2019, 2:47 PM

Bug fix, rebase

paquette added inline comments.Jan 28 2019, 3:39 PM
include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
135

Comment?

lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1795

Maybe we should assert that Size >= NarrowSize?

1839–1840

Can this just be an else if?

1845

Out of curiosity, could NumParts ever be 0 for some reason? If so, what would that mean?

1852

Can we have a comment explaining what this lambda is doing, just for a small mental breather? :P

1859

Offset/8 is used a few places here, maybe make it a variable?

1883

Can we have a comment here?

arsenm updated this revision to Diff 184153.Jan 29 2019, 12:20 PM
arsenm marked 6 inline comments as done.

Add comments, rearrange parameters to insertParts

arsenm marked an inline comment as done.Jan 29 2019, 12:23 PM
arsenm added inline comments.
include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
136

The comment mentions G_MERGE_VALUES, although technically this is introduced in D57310

lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1839–1840

It would work, but I don't think it logically flows that way

1845

That's the invalid Size < NarrowSize case

aemerson accepted this revision.Jan 30 2019, 10:48 AM

LGTM with nit.

lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1869

Alignment?

This revision is now accepted and ready to land.Jan 30 2019, 10:48 AM
arsenm marked an inline comment as done.Jan 30 2019, 10:51 AM
arsenm added inline comments.
lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1869

The alignment isn't used here. The base alignment is copied from the MMO, and the effective alignment is computed from the base alignment + the offset.

arsenm closed this revision.Jan 30 2019, 6:46 PM

r352720