[PATCH] Remap frame variables used in outlined C++ exception handlers

Reid Kleckner rnk at google.com
Thu Feb 19 15:49:02 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:43
@@ +42,3 @@
+// frame allocation structure.
+typedef MapVector<AllocaInst *, std::pair<SmallVector<AllocaInst *, 1>, int>>
+    FrameVarInfoMap;
----------------
SmallVectors don't nest in other containers very efficiently. You can use a TinyPtrVector instead, which just one pointer, and can be moved very cheaply during a container reallocation.

I'm also partial to just making a struct for the map values instead of using std::pair, along the lines of:
  struct HandlerAllocas {
    TinyPtrVector<AllocaInst> Allocas;
    int ParentFrameAllocationIndex;
  };

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:245
@@ +244,3 @@
+  int Idx = 2;
+
+  // Map the alloca instructions to the corresponding index in the
----------------
Can we sort FrameVarInfo by the alignment and size of ParentAlloca first? It doesn't have to be this patch, but a FIXME here would be good. Since we're freezing the stack layout here during preparation, we can't rely on downstream stack layout passes doing it for us. =/

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:256-258
@@ +255,5 @@
+  // problem.
+  for (FrameVarInfoMap::iterator I = FrameVarInfo.begin(),
+                                 E = FrameVarInfo.end();
+       I != E; I++) {
+    AllocaInst *ParentAlloca = I->first;
----------------
Range-based for? I think `for (const auto &KeyValue : FrameVarInfo)` will do the trick.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:328-330
@@ +327,5 @@
+  // that get the equivalent pointer from the frame allocation struct.
+  for (FrameVarInfoMap::iterator I = FrameVarInfo.begin(),
+                                 E = FrameVarInfo.end();
+       I != E; I++) {
+    AllocaInst *ParentAlloca = I->first;
----------------
Range for?

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:347-348
@@ +346,4 @@
+      Builder.SetCurrentDebugLocation(ParentAlloca->getDebugLoc());
+      Value *ElementPtr =
+          Builder.CreateConstInBoundsGEP2_32(FrameEHData, 0, Idx);
+      ParentAlloca->replaceAllUsesWith(ElementPtr);
----------------
Ultimately, to reduce register pressure and improve address mode matching, we'll want to sink these GEPs into the basic blocks of their uses. Normally, we could rely on CodeGenPrepare to do that for us, but it runs before EH preparation, so we can't.

We don't have to do that now, but a FIXME would be good. We should be able to write a helper function that iterates the use-list and inserts a GEP prior to each instruction.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:605-606
@@ +604,4 @@
+
+// FIXME: This doesn't work during cloning because it finds an instruction
+//        in the use list that isn't yet part of a basic block.
+#if 0
----------------
This is pushing me in the direction of trying to do an upfront analysis that identifies blocks in need of outlining instead of discovering blocks as we go.

================
Comment at: test/CodeGen/X86/cppeh-catch-scalar.ll:28
@@ +27,3 @@
+; CHECK: define void @_Z4testv() #0 {
+; CHECKL entry:
+; CHECK:   %frame.alloc = call i8* @llvm.frameallocate(i32 24)
----------------
CHECKL? Is that intentional?

================
Comment at: test/CodeGen/X86/cppeh-catch-scalar.ll:34
@@ +33,3 @@
+; CHECK-NOT:  %i = alloca i32, align 4
+; CHECT:  %i = getelementptr inbounds %struct._Z4testv.ehdata* %eh.data, i32 0, i32 2
+
----------------
Ditto for CHECT

http://reviews.llvm.org/D7770

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list