This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombine] Do not try to deduplicate commutative operations if both operand are the same.
ClosedPublic

Authored by deadalnix on Jun 2 2017, 9:39 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

deadalnix created this revision.Jun 2 2017, 9:39 AM
RKSimon edited edge metadata.Jun 2 2017, 10:17 AM

Do you have any stats?

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
1565 ↗(On Diff #101231)

Why you're here, please can you change this to SDISel to make it clearer.

deadalnix updated this revision to Diff 101261.Jun 2 2017, 12:30 PM

make comment clearer.

Do you have any stats?

Any compile time stats? Compiling clang before/after this patch should be good enough for an initial comparison.

Hi,

I got sidetracked into some other very urgent project during June. I'll do my best to provide that number as soon as I can. I don't expect the gain, as this, to be that great, but who knows ? What I'm after here is to reduce the impact of D33587 on performances.

OK, benchmarks. Compiling clang from a bc containing clang in its entierety. With the patch:

real    9m45.457s
user    9m44.085s
sys     0m1.384s

Without

real    9m45.521s
user    9m44.016s
sys     0m1.517s

The impact on perfs is not very significant. It's within the noise.

nemanjai edited edge metadata.Aug 11 2017, 3:14 PM

I think that as expected, this has a minimal impact on compile time. But of course, it can't hurt I suppose. I don't see why anyone would be opposed to this.

This revision was automatically updated to reflect the committed changes.