[PATCH] D96993: [DCE] Don't remove non-willreturn calls

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: fhahn, jdoerfert.
Herald added a subscriber: hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

In both ADCE and BDCE (via DemandedBits) we should not remove instructions that are not guaranteed to return. This issue was pointed out by @fhahn in the recent llvm-dev thread.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96993

Files:
  llvm/lib/Analysis/DemandedBits.cpp
  llvm/lib/Transforms/Scalar/ADCE.cpp
  llvm/test/Transforms/ADCE/dce_pure_call.ll
  llvm/test/Transforms/ADCE/willreturn.ll
  llvm/test/Transforms/BDCE/dce-pure.ll
  llvm/test/Transforms/BDCE/dead-void-ro.ll
  llvm/test/Transforms/BDCE/willreturn.ll


Index: llvm/test/Transforms/BDCE/willreturn.ll
===================================================================
--- llvm/test/Transforms/BDCE/willreturn.ll
+++ llvm/test/Transforms/BDCE/willreturn.ll
@@ -4,9 +4,10 @@
 declare void @may_not_return(i32) nounwind readnone
 declare void @will_return(i32) nounwind readnone willreturn
 
-; FIXME: This is a miscompile.
 define void @test(i32 %a) {
 ; CHECK-LABEL: @test(
+; CHECK-NEXT:    [[B:%.*]] = add i32 [[A:%.*]], 1
+; CHECK-NEXT:    call void @may_not_return(i32 [[B]])
 ; CHECK-NEXT:    ret void
 ;
   %b = add i32 %a, 1
Index: llvm/test/Transforms/BDCE/dead-void-ro.ll
===================================================================
--- llvm/test/Transforms/BDCE/dead-void-ro.ll
+++ llvm/test/Transforms/BDCE/dead-void-ro.ll
@@ -14,5 +14,5 @@
 
 declare void @no_side_effects_so_dead(i16) #0
 
-attributes #0 = { nounwind readnone }
+attributes #0 = { nounwind readnone willreturn }
 
Index: llvm/test/Transforms/BDCE/dce-pure.ll
===================================================================
--- llvm/test/Transforms/BDCE/dce-pure.ll
+++ llvm/test/Transforms/BDCE/dce-pure.ll
@@ -1,7 +1,7 @@
 ; RUN: opt -bdce -S < %s | FileCheck %s
 ; RUN: opt -passes=bdce -S < %s | FileCheck %s
 
-declare i32 @strlen(i8*) readonly nounwind
+declare i32 @strlen(i8*) readonly nounwind willreturn
 
 define void @test1() {
   call i32 @strlen( i8* null )
Index: llvm/test/Transforms/ADCE/willreturn.ll
===================================================================
--- llvm/test/Transforms/ADCE/willreturn.ll
+++ llvm/test/Transforms/ADCE/willreturn.ll
@@ -4,9 +4,10 @@
 declare void @may_not_return(i32) nounwind readnone
 declare void @will_return(i32) nounwind readnone willreturn
 
-; FIXME: This is a miscompile.
 define void @test(i32 %a) {
 ; CHECK-LABEL: @test(
+; CHECK-NEXT:    [[B:%.*]] = add i32 [[A:%.*]], 1
+; CHECK-NEXT:    call void @may_not_return(i32 [[B]])
 ; CHECK-NEXT:    ret void
 ;
   %b = add i32 %a, 1
Index: llvm/test/Transforms/ADCE/dce_pure_call.ll
===================================================================
--- llvm/test/Transforms/ADCE/dce_pure_call.ll
+++ llvm/test/Transforms/ADCE/dce_pure_call.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -adce -S < %s | not grep call
 
-declare i32 @strlen(i8*) readonly nounwind
+declare i32 @strlen(i8*) readonly nounwind willreturn
 
 define void @test() {
 	call i32 @strlen( i8* null )		; <i32>:1 [#uses=0]
Index: llvm/lib/Transforms/Scalar/ADCE.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/ADCE.cpp
+++ llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -325,7 +325,7 @@
 
 bool AggressiveDeadCodeElimination::isAlwaysLive(Instruction &I) {
   // TODO -- use llvm::isInstructionTriviallyDead
-  if (I.isEHPad() || I.mayHaveSideEffects()) {
+  if (I.isEHPad() || I.mayHaveSideEffects() || !I.willReturn()) {
     // Skip any value profile instrumentation calls if they are
     // instrumenting constants.
     if (isInstrumentsConstant(I))
Index: llvm/lib/Analysis/DemandedBits.cpp
===================================================================
--- llvm/lib/Analysis/DemandedBits.cpp
+++ llvm/lib/Analysis/DemandedBits.cpp
@@ -80,7 +80,7 @@
 
 static bool isAlwaysLive(Instruction *I) {
   return I->isTerminator() || isa<DbgInfoIntrinsic>(I) || I->isEHPad() ||
-         I->mayHaveSideEffects();
+         I->mayHaveSideEffects() || !I->willReturn();
 }
 
 void DemandedBits::determineLiveOperandBits(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96993.324757.patch
Type: text/x-patch
Size: 3482 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210218/ca0c3bc9/attachment-0001.bin>