Proposed patch to prevent the creation of empty (forwarding) blocks resulting from nested ifs.

[wolfgangp]

Summary

Currently nested if statements can generate empty BB's whose terminator simply branches unconditionally to its successor.
These branches are not eliminated by LLVM (r154417) in order to help generate better line number information
in certain cases, specifically empty breaks in switch statements (r154420).

However, there is no need to keep the empty blocks that result from nested ifs around, as there is no corresponding
source construct that would benefit from better line info. The patch proposes to remove such blocks when they result
directly from nested ifs.

Consider the following case:
/******
void func()
{}

int bar(int val)
{

if (val == 0)
{
    func();
}
else if (val == 1)
{
    func();   // line 12
}
return 0;

}

int main()
{

return bar(0);

}
//********
The resulting IR for bar is

...
define i32 @bar(i32 %val) #0 {
entry:

%retval = alloca i32, align 4
%val.addr = alloca i32, align 4
store i32 %val, i32* %val.addr, align 4
%0 = load i32, i32* %val.addr, align 4
%cmp = icmp eq i32 %0, 0
br i1 %cmp, label %if.then, label %if.else

if.then: ; preds = %entry

store i32 1, i32* %retval
br label %return

if.else: ; preds = %entry

%1 = load i32, i32* %val.addr, align 4
%cmp1 = icmp eq i32 %1, 1
br i1 %cmp1, label %if.then.2, label %if.end

if.then.2: ; preds = %if.else

call void (...) @func()
br label %if.end

if.end: ; preds = %if.then.2, %if.else <==============

br label %if.end.3                                                               <==============

if.end.3: ; preds = %if.end

store i32 0, i32* %retval
br label %return

return: ; preds = %if.end.3, %if.then

%2 = load i32, i32* %retval
ret i32 %2

}
...

if.end would be eliminated with the patch applied. The idea is that nested ifs with no else branches will generate an empty block,
which can be eliminated at the outer level.

[wolfgangp]

Ping...

[wolfgangp]

Updating this patch against a recent revision.

[wolfgangp]

Ping...

[mehdi_amini]

lib/CodeGen/CGStmt.cpp

614

Document

634

Document

test/CodeGen/forwarding-blocks-if.c

18

Any reason to not stick with LLVM coding convention here?

[wolfgangp]

Addressed review comments: documented changes and clang-formatted the test case.

test/CodeGen/forwarding-blocks-if.c

18

No reason, thanks for pointing this out.

[mehdi_amini]

LGTM.

[mehdi_amini]

This is committed r275115 for the record.

Phabricator will automatically close it if the last line is "Differential Revision: ..." (You put "Differential review: ..." instead).

lib/CodeGen/CGStmt.cpp

 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);
   incrementProfileCounter(&S);
   {
     RunCleanupsScope ThenScope(*this);
     EmitStmt(S.getThen());
   }
-  EmitBranch(ContBlock);
+  {
+    auto CurBlock = Builder.GetInsertBlock();
+    EmitBranch(ContBlock);
+    // Eliminate any empty blocks that may have been created by nested
+    // control flow statements in the 'then' clause.
+    if (CurBlock)
+      SimplifyForwardingBlocks(CurBlock); 
+  }
 
   // Emit the 'else' code if present.
   if (const Stmt *Else = S.getElse()) {
     {
       // There is no need to emit line number for an unconditional branch.
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
       EmitBlock(ElseBlock);
     }
     {
       RunCleanupsScope ElseScope(*this);
       EmitStmt(Else);
     }
     {
       // There is no need to emit line number for an unconditional branch.
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
+      auto CurBlock = Builder.GetInsertBlock();
       EmitBranch(ContBlock);
+      // Eliminate any empty blocks that may have been created by nested
+      // control flow statements emitted in the 'else' clause.
+      if (CurBlock)
+        SimplifyForwardingBlocks(CurBlock); 
     }
   }
 
   // Emit the continuation block for code after the if.
   EmitBlock(ContBlock, true);
 }
 
 void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,

test/CodeGen/forwarding-blocks-if.c

+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+// Check that no empty blocks are generated for nested ifs.
+
+extern void func();
+
+int f0(int val) {
+  if (val == 0) {
+    func();
+  } else if (val == 1) {
+    func();
+  }
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @f0
+// CHECK: call void {{.*}} @func
+// CHECK: call void {{.*}} @func
+// CHECK: br label %[[RETBLOCK1:[^ ]*]]
+// CHECK: [[RETBLOCK1]]:
+// CHECK-NOT: br label
+// CHECK: ret i32
+
+int f1(int val, int g) {
+  if (val == 0)
+    if (g == 1) {
+      func();
+    }
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @f1
+// CHECK: call void {{.*}} @func
+// CHECK: br label %[[RETBLOCK2:[^ ]*]]
+// CHECK: [[RETBLOCK2]]:
+// CHECK-NOT: br label
+// CHECK: ret i32