Make simdloop marker more robust

Previously the simdloop marker was metadata on a random add instruction in the loop.
This is problematic if that add instruction gets moved around or folded by an earlier
optimization pass. Switching to using an explicit marker instruction. Eventually we
should do something different here to address #26976. This just makes it more robust
to make sure it keeps working with the new optimizer.

src/codegen.cpp

@@ -301,6 +301,7 @@ static Function *jlegal_func;
 static Function *jl_alloc_obj_func;
 static Function *jl_newbits_func;
 static Function *jl_typeof_func;
+static Function *jl_simdloop_marker_func;
 static Function *jl_write_barrier_func;
 static Function *jlisa_func;
 static Function *jlsubtype_func;
@@ -4084,7 +4085,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr)
                     maybe_decay_untracked(boxed(ctx, ast))), true, jl_expr_type);
     }
     else if (head == simdloop_sym) {
-        llvm::annotateSimdLoop(ctx.builder.GetInsertBlock());
+        ctx.builder.CreateCall(prepare_call(jl_simdloop_marker_func));
         return jl_cgval_t();
     }
     else if (head == goto_ifnot_sym) {
@@ -7439,6 +7440,13 @@ static void init_julia_llvm_env(Module *m)
     add_return_attr(jl_newbits_func, Attribute::NonNull);
     add_named_global(jl_newbits_func, (void*)jl_new_bits);
 
+    jl_simdloop_marker_func = Function::Create(FunctionType::get(T_void, {}, false),
+                                               Function::ExternalLinkage,
+                                               "julia.simdloop_marker");
+    jl_simdloop_marker_func->addFnAttr(Attribute::NoUnwind);
+    jl_simdloop_marker_func->addFnAttr(Attribute::NoRecurse);
+    jl_simdloop_marker_func->addFnAttr(Attribute::InaccessibleMemOnly);
+
     jl_typeof_func = Function::Create(FunctionType::get(T_prjlvalue, {T_prjlvalue}, false),
                                       Function::ExternalLinkage,
                                       "julia.typeof");

src/jitlayers.cpp

@@ -134,6 +134,7 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
         PM->add(createLateLowerGCFramePass());
         PM->add(createLowerPTLSPass(dump_native));
 #endif
+        PM->add(createLowerSimdLoopPass());        // Annotate loop marked with "simdloop" as LLVM parallel loop
         if (dump_native)
             PM->add(createMultiVersioningPass());
         return;
@@ -145,8 +146,8 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
     }
     // list of passes from vmkit
     PM->add(createCFGSimplificationPass()); // Clean up disgusting code
-    PM->add(createDeadInstEliminationPass());
-    PM->add(createPromoteMemoryToRegisterPass()); // Kill useless allocas
+    PM->add(createDeadCodeEliminationPass());
+    PM->add(createSROAPass()); // Kill useless allocas
 
     // Due to bugs and missing features LLVM < 5.0, does not properly propagate
     // our invariants. We need to do GC rooting here. This reduces the

src/llvm-simdloop.cpp

@@ -55,16 +55,26 @@ bool annotateSimdLoop(BasicBlock *incr)
     return false;
 }
 
-/// Pass that lowers a loop marked by annotateSimdLoop.
 /// This pass should run after reduction variables have been converted to phi nodes,
 /// otherwise floating-point reductions might not be recognized as such and
 /// prevent SIMDization.
-struct LowerSIMDLoop: public LoopPass {
+struct LowerSIMDLoop : public ModulePass {
     static char ID;
-    LowerSIMDLoop() : LoopPass(ID) {}
+    LowerSIMDLoop() : ModulePass(ID)
+    {
+    }
+
+    protected:
+    void getAnalysisUsage(AnalysisUsage &AU) const override
+    {
+        ModulePass::getAnalysisUsage(AU);
+        AU.addRequired<LoopInfoWrapperPass>();
+        AU.addPreserved<LoopInfoWrapperPass>();
+        AU.setPreservesCFG();
+    }
 
-private:
-    bool runOnLoop(Loop *, LPPassManager &LPM) override;
+    private:
+    bool runOnModule(Module &M) override;
 
     /// Check if loop has "simd_loop" annotation.
     /// If present, the annotation is an MDNode attached to an instruction in the loop's latch.
@@ -160,41 +170,65 @@ void LowerSIMDLoop::enableUnsafeAlgebraIfReduction(PHINode *Phi, Loop *L) const
     }
 }
 
-bool LowerSIMDLoop::runOnLoop(Loop *L, LPPassManager &LPM)
+bool LowerSIMDLoop::runOnModule(Module &M)
 {
-    if (!simd_loop_mdkind) {
-        simd_loop_mdkind = L->getHeader()->getContext().getMDKindID("simd_loop");
-        simd_loop_md = MDNode::get(L->getHeader()->getContext(), ArrayRef<Metadata*>());
-    }
+    Function *simdloop_marker = M.getFunction("julia.simdloop_marker");
 
-    if (!hasSIMDLoopMetadata(L))
+    if (!simdloop_marker)
         return false;
 
-    DEBUG(dbgs() << "LSL: simd_loop found\n");
-    BasicBlock *Lh = L->getHeader();
-    DEBUG(dbgs() << "LSL: loop header: " << *Lh << "\n");
-    MDNode *n = L->getLoopID();
-    if (!n) {
-        // Loop does not have a LoopID yet, so give it one.
-        n = MDNode::get(Lh->getContext(), ArrayRef<Metadata*>(NULL));
-        n->replaceOperandWith(0,n);
-        L->setLoopID(n);
-    }
-    MDNode *m = MDNode::get(Lh->getContext(), ArrayRef<Metadata*>(n));
+    bool Changed = false;
+    std::vector<Instruction*> ToDelete;
+    for (User *U : simdloop_marker->users()) {
+        Instruction *I = cast<Instruction>(U);
+        ToDelete.push_back(I);
+        LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>(*I->getParent()->getParent()).getLoopInfo();
+        Loop *L = LI.getLoopFor(I->getParent());
+        I->removeFromParent();
+        if (!L)
+            continue;
+
+        DEBUG(dbgs() << "LSL: simd_loop found\n");
+        BasicBlock *Lh = L->getHeader();
+        DEBUG(dbgs() << "LSL: loop header: " << *Lh << "\n");
+        MDNode *n = L->getLoopID();
+        if (!n) {
+            // Loop does not have a LoopID yet, so give it one.
+            n = MDNode::get(Lh->getContext(), ArrayRef<Metadata *>(NULL));
+            n->replaceOperandWith(0, n);
+            L->setLoopID(n);
+        }
+
+        assert(L->getLoopID());
 
-    // Mark memory references so that Loop::isAnnotatedParallel will return true for this loop.
-    for(Loop::block_iterator BBI = L->block_begin(), E=L->block_end(); BBI!=E; ++BBI)
-        for (BasicBlock::iterator I = (*BBI)->begin(), EE = (*BBI)->end(); I!=EE; ++I)
-            if (I->mayReadOrWriteMemory())
-                I->setMetadata("llvm.mem.parallel_loop_access", m);
-    assert(L->isAnnotatedParallel());
+        MDNode *m = MDNode::get(Lh->getContext(), ArrayRef<Metadata *>(n));
+
+        // Mark memory references so that Loop::isAnnotatedParallel will return true for this loop.
+        for (BasicBlock *BB : L->blocks()) {
+            for (Instruction &I : *BB) {
+                if (I.mayReadOrWriteMemory()) {
+                    I.setMetadata(LLVMContext::MD_mem_parallel_loop_access, m);
+                }
+            }
+        }
+        assert(L->isAnnotatedParallel());
+
+        // Mark floating-point reductions as okay to reassociate/commute.
+        for (BasicBlock::iterator I = Lh->begin(), E = Lh->end(); I != E; ++I) {
+            if (PHINode *Phi = dyn_cast<PHINode>(I))
+                enableUnsafeAlgebraIfReduction(Phi, L);
+            else
+                break;
+        }
+
+        Changed = true;
+    }
 
-    // Mark floating-point reductions as okay to reassociate/commute.
-    for (BasicBlock::iterator I = Lh->begin(), E = Lh->end(); I!=E; ++I)
-        if (PHINode *Phi = dyn_cast<PHINode>(I))
-            enableUnsafeAlgebraIfReduction(Phi,L);
+    for (Instruction *I : ToDelete)
+        delete I;
+    simdloop_marker->eraseFromParent();
 
-    return true;
+    return Changed;
 }
 
 char LowerSIMDLoop::ID = 0;