Update memory effects of Tapir intrinics; fix lowering of tapir_frame

llvm/include/llvm/IR/Intrinsics.td

@@ -49,6 +49,8 @@ def IntrArgMemOnly : IntrinsicProperty;
 // accessible by the module being compiled. This is a weaker form of IntrNoMem.
 def IntrInaccessibleMemOnly : IntrinsicProperty;
 
+def IntrReadInaccessibleMemOnly : IntrinsicProperty;
+
 // IntrInaccessibleMemOrArgMemOnly -- This intrinsic only accesses memory that
 // its pointer-typed arguments point to or memory that is not accessible
 // by the module being compiled. This is a weaker form of IntrArgMemOnly.
@@ -1388,10 +1390,10 @@ def int_syncregion_start
     : Intrinsic<[llvm_token_ty], [], [IntrArgMemOnly, IntrWillReturn]>;
 
 def int_tapir_runtime_start
-    : Intrinsic<[llvm_token_ty], [], [IntrArgMemOnly, IntrWillReturn]>;
+    : Intrinsic<[llvm_token_ty], [], [IntrInaccessibleMemOnly, IntrWillReturn]>;
 
 def int_tapir_runtime_end
-    : Intrinsic<[], [llvm_token_ty], [IntrArgMemOnly, IntrWillReturn]>;
+    : Intrinsic<[], [llvm_token_ty], [IntrInaccessibleMemOnly, IntrWillReturn]>;
 
 // Intrinsics for taskframes.
 
@@ -1443,7 +1445,7 @@ def int_task_frameaddress
 
 // Return the stack_frame in an OpenCilk function, otherwise null.
 def int_tapir_frame
-    : Intrinsic<[llvm_ptr_ty], [], [IntrWillReturn,IntrReadMem,IntrStrandPure]>;
+    : Intrinsic<[llvm_ptr_ty], [], [IntrWillReturn,IntrReadInaccessibleMemOnly]>;
 
 // Ideally the types would be [llvm_anyptr_ty], [LLVMMatchType<0>]
 // but that does not work, so rely on the front end to insert bitcasts.
@@ -1452,7 +1454,7 @@ def int_hyper_lookup
                 [llvm_ptr_ty, llvm_ptr_ty, llvm_anyint_ty,
                 llvm_ptr_ty, llvm_ptr_ty], [
                   NonNull<RetIndex>, NonNull<ArgIndex<1>>,
-                  IntrWillReturn, IntrReadMem, IntrInaccessibleMemOnly,
+                  IntrWillReturn, IntrReadMem, IntrReadInaccessibleMemOnly,
                   IntrStrandPure, IntrHyperView<ArgIndex<1>>,
                   IntrInjective<ArgIndex<1>>]>; // Dereferenceable,
 

llvm/lib/Analysis/BasicAliasAnalysis.cpp

@@ -1442,10 +1442,10 @@ BasicAAResult::getViewClass(const CallBase *Call, AAQueryInfo &AAQI) {
 }
 
 // This matches llvm.tapir.frame.
-static bool isStrandPureNullary(const CallBase *Call) {
-  if (Call->getNumOperands() != 0)
+static bool isTapirFrame(const CallBase *Call) {
+  if (!isa<IntrinsicInst>(Call))
     return false;
-  return Call->getAttributes().hasFnAttr(Attribute::StrandPure);
+  return cast<IntrinsicInst>(Call)->getIntrinsicID() == Intrinsic::tapir_frame;
 }
 
 // Find an argument with the Injective property.
@@ -1490,10 +1490,8 @@ BasicAAResult::checkInjectiveArguments(const CallBase *C1,
     if (isa<UndefValue>(A1) || isa<UndefValue>(A2))
       continue;
     // Calls to llvm.tapir.frame are equivalent.
-    if (isa<CallBase>(A1) && isa<CallBase>(A2) &&
-        (cast<CallBase>(A1)->getCalledOperand() ==
-         cast<CallBase>(A2)->getCalledOperand()) &&
-        isStrandPureNullary(cast<CallBase>(A1)))
+    if (isa<CallBase>(A1) && isTapirFrame(cast<CallBase>(A1)) &&
+        isa<CallBase>(A2) && isTapirFrame(cast<CallBase>(A2)))
       continue;
     Equal = AliasResult::MayAlias;
   }

llvm/lib/Transforms/Tapir/OpenCilkABI.cpp

@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/CFG.h"
 #include "llvm/Analysis/TapirTaskInfo.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -1121,10 +1122,11 @@ LoopOutlineProcessor *OpenCilkABI::getLoopOutlineProcessor(
 Value *OpenCilkABI::getValidFrame(CallBase *FrameCall, DominatorTree &DT) {
   Function *F = FrameCall->getFunction();
   if (Value *Frame = DetachCtxToStackFrame.lookup(F)) {
-    // Make sure a call to enter_frame dominates this reference
-    // and no call to leave_frame does.  Otherwise return a null
-    // pointer value to mean unknown.  This is correct in most
-    // functions and conservative in complicated functions.
+    // Make sure a call to enter_frame dominates this get_frame call
+    // and no call to leave_frame has potentially been executed.
+    // Otherwise return a null pointer value to mean unknown.
+    // This is correct in most functions and conservative in
+    // complicated functions.
     bool Initialized = false;
     Value *Enter1 = CILKRTS_FUNC(enter_frame_helper).getCallee();
     Value *Enter2 = CILKRTS_FUNC(enter_frame).getCallee();
@@ -1134,13 +1136,19 @@ Value *OpenCilkABI::getValidFrame(CallBase *FrameCall, DominatorTree &DT) {
     Value *Leave4 = CilkParentEpilogue.getCallee();
     for (User *U : Frame->users()) {
       if (CallBase *C = dyn_cast<CallBase>(U)) {
-        if (DT.dominates(C, FrameCall)) {
-          if (Function *Fn = C->getCalledFunction()) {
-            if (Fn == Leave1 || Fn == Leave2 | Fn == Leave3 | Fn == Leave4)
-              return Constant::getNullValue(FrameCall->getType());
-            if (Fn == Enter1 || Fn == Enter2)
-              Initialized = true;
-          }
+        Function *Fn = C->getCalledFunction();
+        if (Fn == nullptr) // indirect function call
+          continue;
+        if (Fn == Enter1 || Fn == Enter2) {
+          if (!Initialized && DT.dominates(C, FrameCall))
+            Initialized = true;
+          continue;
+        }
+        if (Fn == Leave1 || Fn == Leave2 | Fn == Leave3 | Fn == Leave4) {
+          // TODO: ...unless an enter_frame call definitely intervenes.
+          if (isPotentiallyReachable(C, FrameCall, nullptr, &DT, nullptr))
+            return Constant::getNullValue(FrameCall->getType());
+          continue;
         }
       }
     }

llvm/utils/TableGen/CodeGenTarget.cpp

@@ -869,6 +869,8 @@ void CodeGenIntrinsic::setProperty(Record *R) {
     ME &= MemoryEffects::argMemOnly();
   else if (R->getName() == "IntrInaccessibleMemOnly")
     ME &= MemoryEffects::inaccessibleMemOnly();
+  else if (R->getName() == "IntrReadInaccessibleMemOnly")
+    ME &= MemoryEffects::inaccessibleMemOnly(ModRefInfo::Ref);
   else if (R->getName() == "IntrInaccessibleMemOrArgMemOnly")
     ME &= MemoryEffects::inaccessibleOrArgMemOnly();
   else if (R->getName() == "Commutative")