[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