Merge pull request #27945 from JuliaLang/jn/arrayvar-cleanup

more compiler cleanup
JuliaLang · Jul 10, 2018 · 25f91ab · 25f91ab
2 parents 1d55130 + d803472
commit 25f91ab
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 237 deletions.
diff --git a/src/Makefile b/src/Makefile
@@ -21,6 +21,9 @@ FLAGS := \
 ifneq ($(USEMSVC), 1)
 FLAGS += -Wall -Wno-strict-aliasing -fno-omit-frame-pointer -fvisibility=hidden -fno-common \
 		 -Wpointer-arith -Wundef
+ifeq ($(USEGCC),1) # GCC bug #25509 (void)__attribute__((warn_unused_result))
+FLAGS += -Wno-unused-result
+endif
 override CFLAGS += -Wold-style-definition -Wstrict-prototypes -Wc++-compat
 endif
 

diff --git a/src/cgutils.cpp b/src/cgutils.cpp
@@ -4,7 +4,9 @@
 
 static Instruction *tbaa_decorate(MDNode *md, Instruction *load_or_store)
 {
-    load_or_store->setMetadata( llvm::LLVMContext::MD_tbaa, md );
+    load_or_store->setMetadata(llvm::LLVMContext::MD_tbaa, md);
+    if (isa<LoadInst>(load_or_store) && md == tbaa_const)
+        load_or_store->setMetadata(LLVMContext::MD_invariant_load, MDNode::get(md->getContext(), None));
     return load_or_store;
 }
 
@@ -358,20 +360,29 @@ static size_t dereferenceable_size(jl_value_t *jt)
     }
 }
 
+// If given alignment is 0 and LLVM's assumed alignment for a load/store via ptr
+// might be stricter than the Julia alignment for jltype, return the alignment of jltype.
+// Otherwise return the given alignment.
+static unsigned julia_alignment(jl_value_t *jltype)
+{
+    unsigned alignment = jl_datatype_align(jltype);
+    assert(alignment <= JL_HEAP_ALIGNMENT);
+    assert(JL_HEAP_ALIGNMENT % alignment == 0);
+    return alignment;
+}
+
 static inline void maybe_mark_argument_dereferenceable(Argument *A, jl_value_t *jt)
 {
-    auto F = A->getParent();
+    AttrBuilder B;
+    B.addAttribute(Attribute::NonNull);
     // The `dereferencable` below does not imply `nonnull` for non addrspace(0) pointers.
-#if JL_LLVM_VERSION >= 50000
-    F->addParamAttr(A->getArgNo(), Attribute::NonNull);
-#else
-    F->setAttributes(F->getAttributes().addAttribute(jl_LLVMContext, A->getArgNo() + 1,
-                                                     Attribute::NonNull));
-#endif
     size_t size = dereferenceable_size(jt);
-    if (!size)
-        return;
-    F->addDereferenceableAttr(A->getArgNo() + 1, size);
+    if (size) {
+        B.addDereferenceableAttr(size);
+        if (!A->getType()->getPointerElementType()->isSized()) // mimic LLVM Loads.cpp isAligned
+            B.addAlignmentAttr(julia_alignment(jt));
+    }
+    A->addAttrs(B);
 }
 
 static inline Instruction *maybe_mark_load_dereferenceable(Instruction *LI, bool can_be_null,
@@ -1229,19 +1240,6 @@ static Value *emit_bounds_check(jl_codectx_t &ctx, const jl_cgval_t &ainfo, jl_v
     return im1;
 }
 
-// If given alignment is 0 and LLVM's assumed alignment for a load/store via ptr
-// might be stricter than the Julia alignment for jltype, return the alignment of jltype.
-// Otherwise return the given alignment.
-static unsigned julia_alignment(jl_value_t *jltype, unsigned alignment)
-{
-    if (!alignment) {
-        alignment = jl_datatype_align(jltype);
-        assert(alignment <= JL_HEAP_ALIGNMENT);
-        assert(JL_HEAP_ALIGNMENT % alignment == 0);
-    }
-    return alignment;
-}
-
 static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt, Value* dest = NULL, MDNode *tbaa_dest = nullptr, bool isVolatile = false);
 
 static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, jl_value_t *jltype,
@@ -1267,8 +1265,8 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
     //    elt = data;
     //}
     //else {
-        Instruction *load = ctx.builder.CreateAlignedLoad(data, isboxed ?
-            alignment : julia_alignment(jltype, alignment), false);
+        Instruction *load = ctx.builder.CreateAlignedLoad(data, isboxed || alignment ?
+            alignment : julia_alignment(jltype), false);
         if (isboxed)
             load = maybe_mark_load_dereferenceable(load, true, jltype);
         if (tbaa) {
@@ -1316,7 +1314,7 @@ static void typed_store(jl_codectx_t &ctx,
     }
     if (idx_0based)
         data = ctx.builder.CreateInBoundsGEP(r->getType(), data, idx_0based);
-    Instruction *store = ctx.builder.CreateAlignedStore(r, data, isboxed ? alignment : julia_alignment(jltype, alignment));
+    Instruction *store = ctx.builder.CreateAlignedStore(r, data, isboxed || alignment ? alignment : julia_alignment(jltype));
     if (tbaa)
         tbaa_decorate(tbaa, store);
 }
@@ -1637,32 +1635,6 @@ static bool arraytype_constshape(jl_value_t *ty)
             jl_is_long(jl_tparam1(ty)) && jl_unbox_long(jl_tparam1(ty)) != 1);
 }
 
-static void maybe_alloc_arrayvar(jl_codectx_t &ctx, int s)
-{
-    jl_value_t *jt = ctx.slots[s].value.typ;
-    if (arraytype_constshape(jt)) {
-        // TODO: this optimization does not yet work with 1-d arrays, since the
-        // length and data pointer can change at any time via push!
-        // we could make it work by reloading the metadata when the array is
-        // passed to an external function (ideally only impure functions)
-        int ndims = jl_unbox_long(jl_tparam1(jt));
-        jl_value_t *jelt = jl_tparam0(jt);
-        bool isboxed = !jl_array_store_unboxed(jelt);
-        Type *elt = julia_type_to_llvm(jelt);
-        if (type_is_ghost(elt))
-            return;
-        if (isboxed)
-            elt = T_prjlvalue;
-        // CreateAlloca is OK here because maybe_alloc_arrayvar is only called in the prologue setup
-        jl_arrayvar_t &av = (*ctx.arrayvars)[s];
-        av.dataptr = ctx.builder.CreateAlloca(PointerType::get(elt, 0));
-        av.len = ctx.builder.CreateAlloca(T_size);
-        for (int i = 0; i < ndims - 1; i++)
-            av.sizes.push_back(ctx.builder.CreateAlloca(T_size));
-        av.ty = jt;
-    }
-}
-
 static Value *emit_arraysize(jl_codectx_t &ctx, const jl_cgval_t &tinfo, Value *dim)
 {
     Value *t = boxed(ctx, tinfo);
@@ -1674,20 +1646,6 @@ static Value *emit_arraysize(jl_codectx_t &ctx, const jl_cgval_t &tinfo, Value *
             tbaa, T_psize);
 }
 
-static jl_arrayvar_t *arrayvar_for(jl_codectx_t &ctx, jl_value_t *ex)
-{
-    if (ex == NULL)
-        return NULL;
-    if (!jl_is_slot(ex))
-        return NULL;
-    int sl = jl_slot_number(ex) - 1;
-    auto av = ctx.arrayvars->find(sl);
-    if (av != ctx.arrayvars->end())
-        return &av->second;
-    //TODO: ssavalue case
-    return NULL;
-}
-
 static Value *emit_arraysize(jl_codectx_t &ctx, const jl_cgval_t &tinfo, int dim)
 {
     return emit_arraysize(ctx, tinfo, ConstantInt::get(T_int32, dim));
@@ -1726,9 +1684,6 @@ static Value *emit_arraylen_prim(jl_codectx_t &ctx, const jl_cgval_t &tinfo)
 
 static Value *emit_arraylen(jl_codectx_t &ctx, const jl_cgval_t &tinfo, jl_value_t *ex)
 {
-    jl_arrayvar_t *av = arrayvar_for(ctx, ex);
-    if (av != NULL)
-        return ctx.builder.CreateLoad(av->len);
     return emit_arraylen_prim(ctx, tinfo);
 }
 
@@ -1752,17 +1707,11 @@ static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isb
 
 static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, jl_value_t *ex, bool isboxed = false)
 {
-    jl_arrayvar_t *av = arrayvar_for(ctx, ex);
-    if (av!=NULL)
-        return ctx.builder.CreateLoad(av->dataptr);
     return emit_arrayptr(ctx, tinfo, isboxed);
 }
 
 static Value *emit_arraysize(jl_codectx_t &ctx, const jl_cgval_t &tinfo, jl_value_t *ex, int dim)
 {
-    jl_arrayvar_t *av = arrayvar_for(ctx, ex);
-    if (av != NULL && dim <= (int)av->sizes.size())
-        return ctx.builder.CreateLoad(av->sizes[dim - 1]);
     return emit_arraysize(ctx, tinfo, dim);
 }
 
@@ -1795,17 +1744,6 @@ static Value *emit_arrayelsize(jl_codectx_t &ctx, const jl_cgval_t &tinfo)
     return tbaa_decorate(tbaa_const, ctx.builder.CreateLoad(addr));
 }
 
-static void assign_arrayvar(jl_codectx_t &ctx, jl_arrayvar_t &av, const jl_cgval_t &ainfo)
-{
-    Value *aptr = emit_bitcast(ctx,
-        emit_arrayptr(ctx, ainfo),
-        av.dataptr->getType()->getContainedType(0));
-    tbaa_decorate(tbaa_arrayptr, ctx.builder.CreateStore(aptr, av.dataptr));
-    ctx.builder.CreateStore(emit_arraylen_prim(ctx, ainfo), av.len);
-    for (size_t i = 0; i < av.sizes.size(); i++)
-        ctx.builder.CreateStore(emit_arraysize(ctx, ainfo, i + 1), av.sizes[i]);
-}
-
 // Returns the size of the array represented by `tinfo` for the given dimension `dim` if
 // `dim` is a valid dimension, otherwise returns constant one.
 static Value *emit_arraysize_for_unsafe_dim(jl_codectx_t &ctx,
@@ -2289,7 +2227,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
             else {
                 Value *src_ptr = data_pointer(ctx, src);
                 unsigned nb = jl_datatype_size(typ);
-                unsigned alignment = julia_alignment(typ, 0);
+                unsigned alignment = julia_alignment(typ);
                 Value *nbytes = ConstantInt::get(T_size, nb);
                 if (skip) {
                     // TODO: this Select is very bad for performance, but is necessary to work around LLVM bugs with the undef option that we want to use:
@@ -2316,7 +2254,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
         bool allunboxed = for_each_uniontype_small(
                 [&](unsigned idx, jl_datatype_t *jt) {
                     unsigned nb = jl_datatype_size(jt);
-                    unsigned alignment = julia_alignment((jl_value_t*)jt, 0);
+                    unsigned alignment = julia_alignment((jl_value_t*)jt);
                     BasicBlock *tempBB = BasicBlock::Create(jl_LLVMContext, "union_move", ctx.f);
                     ctx.builder.SetInsertPoint(tempBB);
                     switchInst->addCase(ConstantInt::get(T_int8, idx), tempBB);

diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -494,14 +494,6 @@ struct jl_varinfo_t {
     }
 };
 
-// aggregate of array metadata
-typedef struct {
-    Value *dataptr;
-    Value *len;
-    std::vector<Value*> sizes;
-    jl_value_t *ty;
-} jl_arrayvar_t;
-
 struct jl_returninfo_t {
     Function *decl;
     enum CallingConv {
@@ -530,7 +522,6 @@ class jl_codectx_t {
     std::vector<jl_cgval_t> SAvalues;
     std::vector<std::tuple<jl_cgval_t, BasicBlock *, AllocaInst *, PHINode *, jl_value_t *>> PhiNodes;
     std::vector<bool> ssavalue_assigned;
-    std::map<int, jl_arrayvar_t> *arrayvars = NULL;
     jl_module_t *module = NULL;
     jl_method_instance_t *linfo = NULL;
     jl_code_info_t *source = NULL;
@@ -3637,15 +3628,6 @@ static void emit_varinfo_assign(jl_codectx_t &ctx, jl_varinfo_t &vi, jl_cgval_t
     if (rval_info.typ == jl_bottom_type)
         return;
 
-    // add info to arrayvar list
-    if (l && rval_info.isboxed) {
-        // check isboxed in case rval isn't the right type (for example, on a dead branch),
-        // so we don't try to assign it to the arrayvar info
-        jl_arrayvar_t *av = arrayvar_for(ctx, l);
-        if (av != NULL)
-            assign_arrayvar(ctx, *av, rval_info);
-    }
-
     // compute / store tindex info
     if (vi.pTIndex) {
         Value *tindex;
@@ -4204,12 +4186,12 @@ static void emit_cfunc_invalidate(
         }
         else {
             gf_ret = emit_bitcast(ctx, gf_ret, gfrt->getPointerTo());
-            ctx.builder.CreateRet(ctx.builder.CreateAlignedLoad(gf_ret, julia_alignment(astrt, 0)));
+            ctx.builder.CreateRet(ctx.builder.CreateAlignedLoad(gf_ret, julia_alignment(astrt)));
         }
         break;
     }
     case jl_returninfo_t::SRet: {
-        emit_memcpy(ctx, &*gf_thunk->arg_begin(), nullptr, gf_ret, nullptr, jl_datatype_size(astrt), julia_alignment(astrt, 0));
+        emit_memcpy(ctx, &*gf_thunk->arg_begin(), nullptr, gf_ret, nullptr, jl_datatype_size(astrt), julia_alignment(astrt));
         ctx.builder.CreateRetVoid();
         break;
     }
@@ -5056,7 +5038,7 @@ static Function *gen_invoke_wrapper(jl_method_instance_t *lam, const jl_returnin
         if (lty != NULL && !isboxed) {
             theArg = decay_derived(emit_bitcast(ctx, theArg, PointerType::get(lty, 0)));
             if (!lty->isAggregateType()) // keep "aggregate" type values in place as pointers
-                theArg = ctx.builder.CreateAlignedLoad(theArg, julia_alignment(ty, 0));
+                theArg = ctx.builder.CreateAlignedLoad(theArg, julia_alignment(ty));
         }
         assert(dyn_cast<UndefValue>(theArg) == NULL);
         args[idx] = theArg;
@@ -5269,9 +5251,7 @@ static std::unique_ptr<Module> emit_function(
 
     //jl_static_show(JL_STDOUT, (jl_value_t*)ast);
     //jl_printf(JL_STDOUT, "\n");
-    std::map<int, jl_arrayvar_t> arrayvars;
     std::map<int, BasicBlock*> labels;
-    ctx.arrayvars = &arrayvars;
     ctx.module = jl_is_method(lam->def.method) ? lam->def.method->module : lam->def.module;
     ctx.linfo = lam;
     ctx.source = src;
@@ -5703,7 +5683,6 @@ static std::unique_ptr<Module> emit_function(
             continue;
         }
         allocate_local(varinfo, s);
-        maybe_alloc_arrayvar(ctx, i);
     }
 
     std::map<int, int> upsilon_to_phic;
@@ -5826,11 +5805,6 @@ static std::unique_ptr<Module> emit_function(
                 Value *argp = boxed(ctx, theArg);
                 ctx.builder.CreateStore(argp, vi.boxroot);
             }
-            // get arrayvar data if applicable
-            if (arrayvars.find(i) != arrayvars.end()) {
-                jl_arrayvar_t av = arrayvars[i];
-                assign_arrayvar(ctx, av, theArg);
-            }
         }
     }
 
@@ -6160,7 +6134,7 @@ static std::unique_ptr<Module> emit_function(
                     if (returninfo.cc == jl_returninfo_t::SRet) {
                         assert(jl_is_concrete_type(jlrettype));
                         emit_memcpy(ctx, sret, nullptr, retvalinfo, jl_datatype_size(jlrettype),
-                                    julia_alignment(jlrettype, 0));
+                                    julia_alignment(jlrettype));
                     }
                     else { // must be jl_returninfo_t::Union
                         emit_unionmove(ctx, sret, nullptr, retvalinfo, /*skip*/isboxed_union);

diff --git a/src/dump.c b/src/dump.c
@@ -1834,10 +1834,10 @@ static jl_value_t *jl_deserialize_typemap_entry(jl_serializer_state *s)
         jl_deserialize_struct(s, v, 1);
 #ifndef NDEBUG
         if (te->func.value && jl_typeis(te->func.value, jl_method_instance_type)) {
-            assert((te->func.linfo->max_world == 0 &&
+            assert(((te->func.linfo->max_world == 0 &&
                     te->func.linfo->min_world == 1) ||
-                   (te->func.linfo->max_world >= te->max_world &&
-                    te->func.linfo->min_world <= te->min_world) &&
+                    (te->func.linfo->max_world >= te->max_world &&
+                     te->func.linfo->min_world <= te->min_world)) &&
                    "corrupt typemap entry structure");
         }
 #endif

diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp
@@ -367,7 +367,7 @@ static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_va
         return NULL;
     }
 
-    unsigned alignment = julia_alignment(jt, 0);
+    unsigned alignment = julia_alignment(jt);
     Type *ptype = to->getPointerTo();
     if (dest) {
         emit_memcpy(ctx, dest, tbaa_dest, p, x.tbaa, jl_datatype_size(jt), alignment, false);
@@ -1325,15 +1325,5 @@ static Function *boxfunc_llvm(FunctionType *ft, const std::string &cname,
 
 static FunctionType *ft1arg(Type *ret, Type *arg)
 {
-    std::vector<Type*> args1(0);
-    args1.push_back(arg);
-    return FunctionType::get(ret, args1, false);
-}
-
-static FunctionType *ft2arg(Type *ret, Type *arg1, Type *arg2)
-{
-    std::vector<Type*> args2(0);
-    args2.push_back(arg1);
-    args2.push_back(arg2);
-    return FunctionType::get(ret, args2, false);
+    return FunctionType::get(ret, { arg }, false);
 }
diff --git a/src/jloptions.c b/src/jloptions.c
@@ -31,12 +31,12 @@ jl_options_t jl_options = { 0,    // quiet
                             -1,   // banner
                             NULL, // julia_bindir
                             NULL, // julia_bin
-                            NULL, // project
                             NULL, // cmds
                             NULL, // image_file (will be filled in below)
                             NULL, // cpu_target ("native", "core2", etc...)
                             0,    // nprocs
                             NULL, // machine_file
+                            NULL, // project
                             0,    // isinteractive
                             0,    // color
                             JL_OPTIONS_HISTORYFILE_ON, // history file