[PATCH] D22413: [ValueTracking] Teach isSafeToSpeculativelyExecute() about the speculatable attribute

[Hal Finkel via llvm-commits]

hfinkel added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:3120-3125
@@ -3119,4 +3119,8 @@
   }
   case Instruction::Call: {
+    auto *CI = cast<const CallInst>(Inst);
+    const Function *Callee = CI->getCalledFunction();
+    if (Callee && Callee->isSpeculatable())
+      return true;
     if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
       switch (II->getIntrinsicID()) {
----------------
majnemer wrote:
> arsenm wrote:
> > I mean the opposite, that an invoke never has a speculatable 
> IMO, it seems like it would belong more in llvm's Lint analysis than a verifier check.
I agree. We could easily end up with invoked functions being speculatable as an intermediate state (especially if we start inferring the attribute).

Also, it is not clearly a logical contradiction. The user might add the speculatable attribute on a function that might throw because the user does not care if the exception is thrown early. I'm not sure what we would do with that, however, because moving the catch blocks around to speculate the invoke seems unlikely worthwhile.


https://reviews.llvm.org/D22413