Skip to content

Commit 56d7ebe

Browse files
KenoKeno
committed
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 JuliaLang#26976. This just makes it more robust to make sure it keeps working with the new optimizer.
1 parent 243c233 commit 56d7ebe

4 files changed

Lines changed: 85 additions & 38 deletions

File tree

‎src/codegen.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ static Function *jlegal_func;
301301
static Function *jl_alloc_obj_func;
302302
static Function *jl_newbits_func;
303303
static Function *jl_typeof_func;
304+
static Function *jl_simdloop_marker_func;
304305
static Function *jl_write_barrier_func;
305306
static Function *jlisa_func;
306307
static Function *jlsubtype_func;
@@ -4084,7 +4085,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr)
40844085
maybe_decay_untracked(boxed(ctx, ast))), true, jl_expr_type);
40854086
}
40864087
else if (head == simdloop_sym) {
4087-
llvm::annotateSimdLoop(ctx.builder.GetInsertBlock());
4088+
ctx.builder.CreateCall(prepare_call(jl_simdloop_marker_func));
40884089
return jl_cgval_t();
40894090
}
40904091
else if (head == goto_ifnot_sym) {
@@ -7437,6 +7438,13 @@ static void init_julia_llvm_env(Module *m)
74377438
add_return_attr(jl_newbits_func, Attribute::NonNull);
74387439
add_named_global(jl_newbits_func, (void*)jl_new_bits);
74397440

7441+
jl_simdloop_marker_func = Function::Create(FunctionType::get(T_void, {}, false),
7442+
Function::ExternalLinkage,
7443+
"julia.simdloop_marker");
7444+
jl_simdloop_marker_func->addFnAttr(Attribute::NoUnwind);
7445+
jl_simdloop_marker_func->addFnAttr(Attribute::NoRecurse);
7446+
jl_simdloop_marker_func->addFnAttr(Attribute::InaccessibleMemOnly);
7447+
74407448
jl_typeof_func = Function::Create(FunctionType::get(T_prjlvalue, {T_prjlvalue}, false),
74417449
Function::ExternalLinkage,
74427450
"julia.typeof");

‎src/jitlayers.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
134134
PM->add(createLateLowerGCFramePass());
135135
PM->add(createLowerPTLSPass(dump_native));
136136
#endif
137+
PM->add(createLowerSimdLoopPass()); // Annotate loop marked with "simdloop" as LLVM parallel loop
137138
if (dump_native)
138139
PM->add(createMultiVersioningPass());
139140
return;
@@ -145,8 +146,8 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
145146
}
146147
// list of passes from vmkit
147148
PM->add(createCFGSimplificationPass()); // Clean up disgusting code
148-
PM->add(createDeadInstEliminationPass());
149-
PM->add(createPromoteMemoryToRegisterPass()); // Kill useless allocas
149+
PM->add(createDeadCodeEliminationPass());
150+
PM->add(createSROAPass()); // Kill useless allocas
150151

151152
// Due to bugs and missing features LLVM < 5.0, does not properly propagate
152153
// our invariants. We need to do GC rooting here. This reduces the

‎src/llvm-simdloop.cpp‎

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,26 @@ bool annotateSimdLoop(BasicBlock *incr)
5555
return false;
5656
}
5757

58-
/// Pass that lowers a loop marked by annotateSimdLoop.
5958
/// This pass should run after reduction variables have been converted to phi nodes,
6059
/// otherwise floating-point reductions might not be recognized as such and
6160
/// prevent SIMDization.
62-
struct LowerSIMDLoop: public LoopPass {
61+
struct LowerSIMDLoop : public ModulePass {
6362
static char ID;
64-
LowerSIMDLoop() : LoopPass(ID) {}
63+
LowerSIMDLoop() : ModulePass(ID)
64+
{
65+
}
66+
67+
protected:
68+
void getAnalysisUsage(AnalysisUsage &AU) const override
69+
{
70+
ModulePass::getAnalysisUsage(AU);
71+
AU.addRequired<LoopInfoWrapperPass>();
72+
AU.addPreserved<LoopInfoWrapperPass>();
73+
AU.setPreservesCFG();
74+
}
6575

66-
private:
67-
bool runOnLoop(Loop *, LPPassManager &LPM) override;
76+
private:
77+
bool runOnModule(Module &M) override;
6878

6979
/// Check if loop has "simd_loop" annotation.
7080
/// 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
160170
}
161171
}
162172

163-
bool LowerSIMDLoop::runOnLoop(Loop *L, LPPassManager &LPM)
173+
bool LowerSIMDLoop::runOnModule(Module &M)
164174
{
165-
if (!simd_loop_mdkind) {
166-
simd_loop_mdkind = L->getHeader()->getContext().getMDKindID("simd_loop");
167-
simd_loop_md = MDNode::get(L->getHeader()->getContext(), ArrayRef<Metadata*>());
168-
}
175+
Function *simdloop_marker = M.getFunction("julia.simdloop_marker");
169176

170-
if (!hasSIMDLoopMetadata(L))
177+
if (!simdloop_marker)
171178
return false;
172179

173-
DEBUG(dbgs() << "LSL: simd_loop found\n");
174-
BasicBlock *Lh = L->getHeader();
175-
DEBUG(dbgs() << "LSL: loop header: " << *Lh << "\n");
176-
MDNode *n = L->getLoopID();
177-
if (!n) {
178-
// Loop does not have a LoopID yet, so give it one.
179-
n = MDNode::get(Lh->getContext(), ArrayRef<Metadata*>(NULL));
180-
n->replaceOperandWith(0,n);
181-
L->setLoopID(n);
182-
}
183-
MDNode *m = MDNode::get(Lh->getContext(), ArrayRef<Metadata*>(n));
180+
bool Changed = false;
181+
std::vector<Instruction*> ToDelete;
182+
for (User *U : simdloop_marker->users()) {
183+
Instruction *I = cast<Instruction>(U);
184+
ToDelete.push_back(I);
185+
LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>(*I->getParent()->getParent()).getLoopInfo();
186+
Loop *L = LI.getLoopFor(I->getParent());
187+
I->removeFromParent();
188+
if (!L)
189+
continue;
190+
191+
DEBUG(dbgs() << "LSL: simd_loop found\n");
192+
BasicBlock *Lh = L->getHeader();
193+
DEBUG(dbgs() << "LSL: loop header: " << *Lh << "\n");
194+
MDNode *n = L->getLoopID();
195+
if (!n) {
196+
// Loop does not have a LoopID yet, so give it one.
197+
n = MDNode::get(Lh->getContext(), ArrayRef<Metadata *>(NULL));
198+
n->replaceOperandWith(0, n);
199+
L->setLoopID(n);
200+
}
201+
202+
assert(L->getLoopID());
184203

185-
// Mark memory references so that Loop::isAnnotatedParallel will return true for this loop.
186-
for(Loop::block_iterator BBI = L->block_begin(), E=L->block_end(); BBI!=E; ++BBI)
187-
for (BasicBlock::iterator I = (*BBI)->begin(), EE = (*BBI)->end(); I!=EE; ++I)
188-
if (I->mayReadOrWriteMemory())
189-
I->setMetadata("llvm.mem.parallel_loop_access", m);
190-
assert(L->isAnnotatedParallel());
204+
MDNode *m = MDNode::get(Lh->getContext(), ArrayRef<Metadata *>(n));
205+
206+
// Mark memory references so that Loop::isAnnotatedParallel will return true for this loop.
207+
for (BasicBlock *BB : L->blocks()) {
208+
for (Instruction &I : *BB) {
209+
if (I.mayReadOrWriteMemory()) {
210+
I.setMetadata(LLVMContext::MD_mem_parallel_loop_access, m);
211+
}
212+
}
213+
}
214+
assert(L->isAnnotatedParallel());
215+
216+
// Mark floating-point reductions as okay to reassociate/commute.
217+
for (BasicBlock::iterator I = Lh->begin(), E = Lh->end(); I != E; ++I) {
218+
if (PHINode *Phi = dyn_cast<PHINode>(I))
219+
enableUnsafeAlgebraIfReduction(Phi, L);
220+
else
221+
break;
222+
}
223+
224+
Changed = true;
225+
}
191226

192-
// Mark floating-point reductions as okay to reassociate/commute.
193-
for (BasicBlock::iterator I = Lh->begin(), E = Lh->end(); I!=E; ++I)
194-
if (PHINode *Phi = dyn_cast<PHINode>(I))
195-
enableUnsafeAlgebraIfReduction(Phi,L);
227+
for (Instruction *I : ToDelete)
228+
I->deleteValue();
229+
simdloop_marker->eraseFromParent();
196230

197-
return true;
231+
return Changed;
198232
}
199233

200234
char LowerSIMDLoop::ID = 0;

‎test/llvmpasses/simdloop.ll‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
; RUN: opt -load libjulia%shlibext -LowerSIMDLoop -S %s | FileCheck %s
22

3+
declare void @julia.simdloop_marker()
4+
35
define void @simd_test(double *%a, double *%b) {
46
top:
57
br label %loop
@@ -12,7 +14,8 @@ loop:
1214
%bval = load double, double *%aptr
1315
%cval = fadd double %aval, %bval
1416
store double %cval, double *%bptr
15-
%nexti = add i64 %i, 1, !simd_loop !1
17+
%nexti = add i64 %i, 1
18+
call void @julia.simdloop_marker()
1619
%done = icmp sgt i64 %nexti, 500
1720
br i1 %done, label %loopdone, label %loop
1821
loopdone:
@@ -30,7 +33,8 @@ loop:
3033
%aval = load double, double *%aptr
3134
%nextv = fsub double %v, %aval
3235
; CHECK: fsub fast double %v, %aval
33-
%nexti = add i64 %i, 1, !simd_loop !1
36+
%nexti = add i64 %i, 1
37+
call void @julia.simdloop_marker()
3438
%done = icmp sgt i64 %nexti, 500
3539
br i1 %done, label %loopdone, label %loop
3640
loopdone:

0 commit comments

Comments
 (0)