[PATCH] D108657: [CodeExtractor] Ensuring ordering of exitStub creation

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1268
+
+    Value *brVal = nullptr;
+    switch (NumExitBlocks) {
----------------
Should there be an assert:

```
assert(NumExitBlocks < 0xffff && "too many exit blocks for switch");
```

or do you think that's too unlikely to matter?


================
Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1283
+    // Update the switch instruction.
+    TheSwitch->addCase(ConstantInt::get(Type::getInt16Ty(Context),
+                                        SuccNum),
----------------
Doesn't the switch need the same type as the cases, or am I misunderstanding what's going on here?


================
Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1294
         // add a new basic block which returns the appropriate value
         BasicBlock *&NewTarget = ExitBlockMap[OldTarget];
+        assert(NewTarget && "Unknown target block!");
----------------
Code following this doesn't assign into the variable, so we may as well drop the `&` on this one for clarity.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108657/new/

https://reviews.llvm.org/D108657