CGCleanup: No need to do domination fixups for static allocas

[GorNishanov]

Summary

This is a follow up for the rL297084 which made sure that dominance is preserved during expression cleanup emission.
We ran into a case where dominance fixup code was inserting the store in the middle of allocas.

%x = alloca i32, align 4
store i32* %x, i32** %tmp.exprcleanup, align 4 ; <===== HERE
%ref.tmp3 = alloca %struct.A, align 1
%agg.tmp5 = alloca %"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 4
%tmp.exprcleanup = alloca i32*, align 4
%allocapt = bitcast i32 undef to i32
store i32 %0, i32* %.addr, align 4

This fix make sure that domination fixup is not done static allocas.

(Alternative of https://reviews.llvm.org/D33663)

[rnk]

test/CodeGenCoroutines/coro-await-domination.cpp

33

It looks like this is the expression in question. This expression should have aggregate evaluation kind, not scalar. We don't need to reload aggregate expression evaluation results because they are represented with temporary allocas, and they don't have dominance problems. It seems like there is an incorrect call to PopCleanupBlocks somewhere, and that's where the real fix should be.

[rnk]

test/CodeGenCoroutines/coro-await-domination.cpp

33

OK, nevermind. I found a way to reproduce this issue with gnu statement expressions, and after trying to fix it at a higher level, I decided the way you are fixing it is better. I went ahead and landed the fix along with my test cases. Please go ahead and land the coroutines test, though. More coverage is better.

[GorNishanov]

Tested checked in. Thank you for looking into this, Reid!

r304380 = 1c0d15a4bf10876bb233e921907e114d52bee82e

lib/CodeGen/CGCleanup.cpp

if (!HadBranches)
     return;
 
   // Spill and reload all values that the caller wants to be live at the current
   // insertion point.
   for (llvm::Value **ReloadedValue : ValuesToReload) {
     auto *Inst = dyn_cast_or_null<llvm::Instruction>(*ReloadedValue);
     if (!Inst)
       continue;
+    // If it is a static alloca, no need to spill.
+    if (auto *AI = dyn_cast<llvm::AllocaInst>(Inst))
+      if (AI->isStaticAlloca())
+        continue;
+
     Address Tmp =
         CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
 
     // Find an insertion point after Inst and spill it to the temporary.
     llvm::BasicBlock::iterator InsertBefore;
     if (auto *Invoke = dyn_cast<llvm::InvokeInst>(Inst))
       InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
     else

test/CodeGenCoroutines/coro-await-domination.cpp

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+  struct promise_type {
+    coro get_return_object();
+    suspend_never initial_suspend();
+    suspend_never final_suspend();
+    void return_void();
+    static void unhandled_exception();
+  };
+};
+
+struct A {
+  ~A();
+  bool await_ready();
+  int await_resume();
+  template <typename F> void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+//  %tmp.exprcleanup = alloca i32*, align 8
+//  store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+  int x = 42;
+  x = co_await A{};
+}