[PATCH] D23929: GVN-hoist: only hoist relevant scalar instructions

[Sebastian Pop via llvm-commits]

sebpop created this revision.
sebpop added a reviewer: dberlin.
sebpop added a subscriber: llvm-commits.

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.

https://reviews.llvm.org/D23929

Files:
  llvm/lib/Transforms/Scalar/GVNHoist.cpp

Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -874,6 +874,15 @@
     return {NI, NL + NC + NS};
   }
 
+  // Return true when I is a scalar candidate for hoisting.
+  bool isRelevantForHoisting(Instruction *I) {
+    if (isa<TerminatorInst>(I) || isa<FenceInst>(I) || isa<AllocaInst>(I) ||
+        isa<CleanupPadInst>(I) || isa<VAArgInst>(I) || isa<AtomicRMWInst>(I) ||
+        isa<LandingPadInst>(I) || isa<AtomicCmpXchgInst>(I))
+      return false;
+    return true;
+  }
+
   // Hoist all expressions. Returns Number of scalars hoisted
   // and number of non-scalars hoisted.
   std::pair<unsigned, unsigned> hoistExpressions(Function &F) {
@@ -916,7 +925,8 @@
           // that could result in spills later. geps are handled separately.
           // TODO: We can relax this for targets like AArch64 as they have more
           // registers than X86.
-          II.insert(&I1, VN);
+          if (isRelevantForHoisting(&I1))
+            II.insert(&I1, VN);
       }
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23929.69387.patch
Type: text/x-patch
Size: 1146 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160826/a9b53dc0/attachment.bin>