[PATCH] D15517: [WinEH] Use operand bundles to describe call sites

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 14 20:58:44 PST 2015


sanjoy added a comment.

Operand bundles bits lgtm with some minor comments.  Doesn't have to be this patch, but please add a small blurb to the langref about the `"funclet"` operand bundle.


================
Comment at: lib/CodeGen/WinEHPrepare.cpp:795
@@ +794,3 @@
+        Value *FuncletBundleOperand = nullptr;
+        if (Optional<OperandBundleUse> BU =
+                II->getOperandBundle(LLVMContext::OB_funclet))
----------------
Minor: will this look cleaner with an `auto`?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1446
@@ +1445,3 @@
+        ArrayRef<Value *> Inputs = CallSiteEHPad;
+        OpBundles.emplace_back("funclet", Inputs);
+
----------------
Why do you need to declare `Inputs` separately?  Why not just `OpBundles.emplace_back("funclet", CallSiteEHPad);`?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1449
@@ +1448,3 @@
+        Instruction *NewInst;
+        if (CS.isCall()) {
+          NewInst = CallInst::Create(cast<CallInst>(I), OpBundles, I);
----------------
Minor: braces unnecessary.  This may even be clearer as a ternary operator.


http://reviews.llvm.org/D15517





More information about the llvm-commits mailing list