Page MenuHomePhabricator

[SeparateConstOffsetFromGEP] Bail there is only one GEP offset
AbandonedPublic

Authored by jingyue on Apr 21 2015, 6:54 AM.

Details

Reviewers
jmolloy
Gerolf
Summary

A single-offset GEP such as:

getelementptr i8*, i8* %base, i32 %offset

doesn't make sense to split apart. There is no complex calculation to CSE.

Diff Detail

Event Timeline

jmolloy updated this revision to Diff 24122.Apr 21 2015, 6:54 AM
jmolloy retitled this revision from to [SeparateConstOffsetFromGEP] Bail there is only one GEP offset.
jmolloy updated this object.
jmolloy edited the test plan for this revision. (Show Details)
jmolloy added reviewers: jingyue, wu, Gerolf.
jmolloy set the repository for this revision to rL LLVM.
jmolloy added a subscriber: Unknown Object (MLST).
jingyue edited edge metadata.Apr 21 2015, 9:29 AM

Hi James,

I don't get why

getelementptr i8*, i8* %base, i32 %offset

is not worth splitting if %offset can be reassociated to variadic+const. base+variadic may still be CSE'ed in that case.

For example,

p1 = &base[offset + 1];
p2 = &base[offset + 2];

Reassociating GEPs gives us

p1 = (&base[offset]) + 1
p2 = (&base[offset]) + 2

where &base[offset] can be CSE'ed.

Can you show me the code patterns you discovered that demonstrates the
regression? I can think about how to address them too. Thanks!

Jingyue

wu resigned from this revision.Apr 21 2015, 6:56 PM
wu removed a reviewer: wu.
jingyue commandeered this revision.Mar 15 2016, 10:31 AM
jingyue edited reviewers, added: jmolloy; removed: jingyue.
Gerolf added inline comments.Mar 15 2016, 10:47 AM
lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
832

Please comment on the rationale for ==2. Is there a more general cost model problem?

jingyue abandoned this revision.Mar 15 2016, 10:52 AM

Oops... I pressed the wrong button. I meant to say "need revision". jmolly@, feel free to reclaim it.