[PATCH] D38856: [IPSCCP] Remove calls without side effects

[Chris Bieneman via Phabricator via llvm-commits]

beanz updated this revision to Diff 126414.
beanz added a comment.

Abstracting check for whether or not it is safe to remove an instruction as per the feedback from davide.

None of the other feedback seems to have consensus. Anything else I should change here or can this land?

Thanks!


https://reviews.llvm.org/D38856

Files:
  include/llvm/IR/Instruction.h
  lib/IR/Instruction.cpp
  lib/Transforms/Scalar/SCCP.cpp
  test/Transforms/IPConstantProp/remove-call-inst.ll
  test/Transforms/IPConstantProp/user-with-multiple-uses.ll


Index: test/Transforms/IPConstantProp/user-with-multiple-uses.ll
===================================================================
--- test/Transforms/IPConstantProp/user-with-multiple-uses.ll
+++ test/Transforms/IPConstantProp/user-with-multiple-uses.ll
@@ -15,7 +15,7 @@
   ret i32 %call2
 }
 
-define internal i32 @wwrite(i64 %i) nounwind readnone {
+define internal i32 @wwrite(i64 %i) nounwind {
 entry:
   switch i64 %i, label %sw.default [
     i64 3, label %return
@@ -30,5 +30,4 @@
 }
 
 ; CHECK: attributes #0 = { noreturn nounwind }
-; CHECK: attributes #1 = { nounwind readnone }
-; CHECK: attributes [[NUW]] = { nounwind }
+; CHECK: attributes #1 = { nounwind }
Index: test/Transforms/IPConstantProp/remove-call-inst.ll
===================================================================
--- test/Transforms/IPConstantProp/remove-call-inst.ll
+++ test/Transforms/IPConstantProp/remove-call-inst.ll
@@ -6,7 +6,7 @@
 
 ; CHECK: define i32 @main() #0 {
 ; CHECK-NEXT: entry:
-; CHECK-NEXT: %call2 = tail call i32 @wwrite(i64 0) [[NUW:#[0-9]+]]
+; CHECK-NOT: call
 ; CHECK-NEXT: ret i32 123
 
 define i32 @main() noreturn nounwind {
@@ -31,4 +31,3 @@
 
 ; CHECK: attributes #0 = { noreturn nounwind }
 ; CHECK: attributes #1 = { nounwind readnone }
-; CHECK: attributes [[NUW]] = { nounwind }
Index: lib/Transforms/Scalar/SCCP.cpp
===================================================================
--- lib/Transforms/Scalar/SCCP.cpp
+++ lib/Transforms/Scalar/SCCP.cpp
@@ -1902,7 +1902,7 @@
         if (Inst->getType()->isVoidTy())
           continue;
         if (tryToReplaceWithConstant(Solver, Inst)) {
-          if (!isa<CallInst>(Inst) && !isa<TerminatorInst>(Inst))
+          if (Inst->isSafeToRemove())
             Inst->eraseFromParent();
           // Hey, we just changed something!
           MadeChanges = true;
Index: lib/IR/Instruction.cpp
===================================================================
--- lib/IR/Instruction.cpp
+++ lib/IR/Instruction.cpp
@@ -591,6 +591,11 @@
   return isa<ResumeInst>(this);
 }
 
+bool Instruction::isSafeToRemove() const {
+  return (!isa<CallInst>(this) || !this->mayHaveSideEffects()) &&
+         !isa<TerminatorInst>(this);
+}
+
 bool Instruction::isAssociative() const {
   unsigned Opcode = getOpcode();
   if (isAssociative(Opcode))
Index: include/llvm/IR/Instruction.h
===================================================================
--- include/llvm/IR/Instruction.h
+++ include/llvm/IR/Instruction.h
@@ -534,6 +534,14 @@
   /// matters, isSafeToSpeculativelyExecute may be more appropriate.
   bool mayHaveSideEffects() const { return mayWriteToMemory() || mayThrow(); }
 
+  /// Return true if the instruction can be removed if the result is unused.
+  ///
+  /// When constant folding some instructions cannot be removed even if their
+  /// results are unused. Specifically terminator instructions and calls that
+  /// may have side effects cannot be removed without semantically changing the
+  /// generated program.
+  bool isSafeToRemove() const;
+  
   /// Return true if the instruction is a variety of EH-block.
   bool isEHPad() const {
     switch (getOpcode()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38856.126414.patch
Type: text/x-patch
Size: 3163 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171211/acf75ca9/attachment.bin>