This is an archive of the discontinued LLVM Phabricator instance.

GVN-hoist: only hoist relevant scalar instructions
ClosedPublic

Authored by sebpop on Aug 26 2016, 9:20 AM.

Details

Summary

Without this patch, GVN-hoist would think that a branch instruction is a scalar instruction
and would try to value number it. The patch filters out all such kind of irrelevant instructions.

A bit frustrating is that there is no easy way to discard all those very infrequent instructions,
a bit like isa<TerminatorInst> that stands for a large family of instructions. I'm thinking that
checking for those very infrequent other instructions would cost us more in compilation time
than just letting those instructions getting numbered, so I'm still thinking that a simpler check:

if (isa<TerminatorInst>(I))
  return false;

is better than listing all the other less frequent instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

sebpop updated this revision to Diff 69387.Aug 26 2016, 9:20 AM
sebpop retitled this revision from to GVN-hoist: only hoist relevant scalar instructions.
sebpop updated this object.
sebpop added a reviewer: dberlin.
sebpop added a subscriber: llvm-commits.
sebpop updated this revision to Diff 71873.Sep 19 2016, 1:37 PM

Updated patch to only filter out TerminatorInst.
As I was mentioning, the other instructions are rare, and we would spend more time in isa<>() than allowing these instructions to be discarded after computing a VN.

This revision was automatically updated to reflect the committed changes.