This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Improve zextload optimization.
ClosedPublic

Authored by gberry on Nov 4 2015, 11:02 AM.

Details

Summary

Don't fold

(zext (and (load x), cst)) -> (and (zextload x), (zext cst))

if

(and (load x) cst)

will match as a zextload already and has additional users.

For example, the following IR:

    
%load = load i32, i32* %ptr, align 8
%load16 = and i32 %load, 65535
%load64 = zext i32 %load16 to i64
store i32 %load16, i32* %dst1, align 4
store i64 %load64, i64* %dst2, align 8

used to produce the following aarch64 code:

	ldr		w8, [x0]
	and	w9, w8, #0xffff
	and	x8, x8, #0xffff
	str		w9, [x1]
	str		x8, [x2]

but with this change produces the following aarch64 code:

	ldrh		w8, [x0]
	str		w8, [x1]
	str		x8, [x2]

Diff Detail

Event Timeline

gberry updated this revision to Diff 39219.Nov 4 2015, 11:02 AM
gberry retitled this revision from to [DAGCombiner] Improve zextload optimization..
gberry updated this object.
gberry added reviewers: resistor, mcrosier.
gberry added a subscriber: llvm-commits.
arsenm added a subscriber: arsenm.Nov 4 2015, 11:57 AM
arsenm added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3007–3009

Should this check TLI,shouldReduceLoadWidth? It maybe should go along with the other tests before the place this is called

gberry added inline comments.Nov 4 2015, 12:22 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3007–3009

That seems like it should be a separate change to me. The intent of this change is not to change the behavior of visitAND at all.

gberry updated this revision to Diff 39237.Nov 4 2015, 12:24 PM

Rename variable WidenLoad to NarrowLoad since that is more accurate.

gberry updated this revision to Diff 39837.Nov 10 2015, 11:19 AM

Ping? Updated change description with an example of improved generated code.

gberry updated this object.Nov 10 2015, 11:21 AM
mcrosier accepted this revision.Nov 11 2015, 8:59 AM
mcrosier edited edge metadata.

LGTM with a few nits.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
423

Perhaps, "(and (load x) cst)" to be consistent with documentation elsewhere?

3003

Maybe add a comment about what's going on here? Basically, no need to narrow the load for the pattern to match..

3007

You can simplify the below conditional as well as improve readability by hoisting out the volatile check.

if (LoadN->isVolatile())

return false;
This revision is now accepted and ready to land.Nov 11 2015, 8:59 AM
gberry updated this revision to Diff 39941.Nov 11 2015, 10:07 AM
gberry updated this object.
gberry edited edge metadata.

Updated to address Chad's comments.

gberry updated this revision to Diff 39951.Nov 11 2015, 11:27 AM

clang-format fixes

This revision was automatically updated to reflect the committed changes.