[llvm-dev] is this a bug in PruneEH?

[Snider, Todd via llvm-dev]

Update ...

It is legal to insert an unreachable after the call to a noreturn function in the if block, but the optimizer should not discard the call in a later optimization pass.

When the inliner pass is run after PruneEH, the call instruction is deemed to have no side effects and is removed.

If I add a noinline attribute to the definition of the noreturn function, the call is again deemed to have no side effects and is removed.

If I put an inline asm nop in the noreturn function, then calls to it are no longer considered trivially dead.

Is a call to a function that contains only an empty "while (1) ;" loop considered dead code? I don't think it should be.

One use of such a function in an embedded system is as a "parking" function. For example, if an error condition occurs at run time which requires service from a regularly occurring interrupt handler, the code that detects the error condition may "park" a thread in an endless loop function to wait on the interrupt handler.

From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Snider, Todd via llvm-dev
Sent: Tuesday, June 4, 2019 10:12 AM
To: llvm-dev
Subject: [EXTERNAL] [llvm-dev] is this a bug in PruneEH?


I have the following C source:

extern unsigned int donut;
void f1();
void f2();

void f1()
{
  unsigned int *magic = &donut;
  if (*magic != 286529877) {
    f2();
  }
}

void f2() {
/* Loop here forever if application is built with wrong version of ROM image */
  while(1) {
    ;
  }
}

The -O2 level PruneEH pass uses SimplifyFunction() which contains this code:

    for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; )
      if (CallInst *CI = dyn_cast<CallInst>(I++))
        if (CI->doesNotReturn() && !CI->isMustTailCall() &&
            !isa<UnreachableInst>(I)) {
          // This call calls a function that cannot return.  Insert an
          // unreachable instruction after it and simplify the code.  Do this
          // by splitting the BB, adding the unreachable, then deleting the
          // new BB.
          BasicBlock *New = BB->splitBasicBlock(I);

          // Remove the uncond branch and add an unreachable.
          BB->getInstList().pop_back();
          new UnreachableInst(BB->getContext(), &*BB);

          DeleteBasicBlock(New, CG);  // Delete the new BB.
          MadeChange = true;
          ++NumUnreach;
          break;
        }

The nested if in SimplifyFunction() will essentially eliminate the call to f2() in the definition of f1() in the source that is being compiled. Here is the before and after pass IR dumps:

*** IR Dump Before Remove unused exception handling info ***
; Function Attrs: nounwind
define hidden void @f1() local_unnamed_addr #0 {
  %1 = load i32, i32* @donut, align 4, !tbaa !3
  %2 = icmp eq i32 %1, 286529877
  br i1 %2, label %4, label %3

3:                                                ; preds = %0
  call void @f2()
  br label %4

4:                                                ; preds = %0, %3
  ret void
}

*** IR Dump After Remove unused exception handling info ***
; Function Attrs: nounwind
define hidden void @f1() local_unnamed_addr #0 {
  %1 = load i32, i32* @donut, align 4, !tbaa !3
  %2 = icmp eq i32 %1, 286529877
  br i1 %2, label %4, label %3

3:                                                ; preds = %0
  call void @f2()
  unreachable

4:                                                ; preds = %0
  ret void
}

If the call to f2() is the only thing in the if block, then inserting an unreachable after it will later be interpreted as an unreachable block.

Is this a legal optimization?

~ Todd Snider
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190604/13dd98d2/attachment.html>