[InstructionCombining] prepareICWorklistFromFunction halts in infinite loop with instructions of token type

[• chenli]

Summary

This patch fixes a bug in prepareICWorklistFromFunction, where the loop becomes infinite with instructions of token type. The patch checks if the instruction is token type, and if so it updates EndInst with the current instruction.

[majnemer]

lib/Transforms/InstCombine/InstructionCombining.cpp

3022–3032 ↗

(On Diff #43899)

Would anything go wrong if we simply had:

if (!Inst->use_empty())
  Inst->replaceAllUsesWith(UndefValue::get(Inst->getType()));
if (Inst->isEHPad()) {
  EndInst = Inst;
  continue;
}
if (!isa<DbgInfoIntrinsic>(Inst)) {
  ++NumDeadInst;
  MadeIRChange = true;
}

Inst->eraseFromParent();

[• chenli]

lib/Transforms/InstCombine/InstructionCombining.cpp

3022–3032 ↗

(On Diff #43899)

It will actually hit a verifier assertion failure if gc.relocate instruction takes an undef gc.statepoint token. But the verifier could be fixed in a later patch if we'd like to have the code you suggested. I dont know if there's similar issues for uses of cleanuppad or catchpad.

[majnemer]

LGTM

llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
     // Delete the instructions backwards, as it has a reduced likelihood of
     // having to update as many def-use and use-def chains.
     Instruction *EndInst = BB->getTerminator(); // Last not to be deleted.
     while (EndInst != BB->begin()) {
       // Delete the next to last instruction.
       Instruction *Inst = &*--EndInst->getIterator();
       if (!Inst->use_empty() && !Inst->getType()->isTokenTy())
         Inst->replaceAllUsesWith(UndefValue::get(Inst->getType()));
-      if (Inst->isEHPad()) {
+      if (Inst->isEHPad() || Inst->getType()->isTokenTy()) {
         EndInst = Inst;
         continue;
       }
       if (!isa<DbgInfoIntrinsic>(Inst)) {
         ++NumDeadInst;
         MadeIRChange = true;
       }
-      if (!Inst->getType()->isTokenTy())
-        Inst->eraseFromParent();
     }
   }
 
   return MadeIRChange;
 }
 
 static bool
 combineInstructionsOverFunction(Function &F, InstCombineWorklist &Worklist,

llvm/trunk/test/Transforms/InstCombine/token.ll

unreachable:
   unreachable
 }
 
 ; CHECK-LABEL: define void @test3(
 ; CHECK:  %X = zext i8 %A to i32
 ; CHECK:  %Y = zext i8 %B to i32
 ; CHECK:  %phi = phi i32 [ %X, %bb ], [ %Y, %cont ], [ %Y, %cont2 ]
 
+declare void @foo()
+declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
+
+define void @test4(i8 addrspace(1)* %obj) gc "statepoint-example" {
+bb:
+  unreachable
+
+unreachable:
+  call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
+  ret void
+}
+
+; CHECK-LABEL: define void @test4(
+; CHECK: unreachable:
+; CHECK:   call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
+; CHECK:   ret void
+
 
 declare void @g(i32)