I will review this. THH ________________________________________ From: Michael Ferguson [[email protected]] Sent: Tuesday, April 01, 2014 12:09 PM To: chapel-developers Subject: [Chapel-developers] request for review: fix LLVM TBAA
Fix TBAA metadata emission in LLVM backend (<100 lines changed, but LLVM-specific) The LLVM backend emits TBAA (Type-Based Alias Analysis) metadata in order to improve optimization. Unfortunately, the code emitting the TBAA was broken in several ways. This patch addresses the problems. The test studies/shootout/nbody/sidelnik/nbody_record_2.chpl revealed the problems and now passes. The LLVM TBAA infrastructure uses a tree of metadata nodes. Ancestors or descendants of a node are considered to possibly alias. Load and store instructions are tagged with the appropriate metadata node. The previous implementation created metadata nodes for records that were leaf nodes (saying that records don't alias with anything other than themselves). In the failing nbody test, there was a metadata node for Planet. A copy implementing b2 = B(j) was thusly marked with the TBAA type Planet. But the load for b2.x reads a real from the same address. Since the load for b2.x was marked with the TBAA type real, and real is not an ancestor or descendant of Planet in the TBAA metadata tree, the load for b2.x and the store to b2 were determined not to alias - and the load for b2.x was hoisted out of the loop. The solution is to not emit TBAA metadata for structure loads/stores/copies, unless using a type of metadata that is aware of the fields of the structure. So we continue to emit tbaa.struct metadata for memcpys. In addition, the patch fixes a few small bugs. compiler/util/llvmUtil.cpp fix an error in converting from bits to bytes compiler/AST/symbol.cpp do not create simple TBAA nodes for types that are struct-like; only create tbaa.struct metadata for these (if appropriate). compiler/AST/expr.cpp in codegenLoadLLVM/codegenStoreLLVM, fix these to use the ptr.chplType in cases when ptr.isLVPtr is set; in those cases, ptr.chplType is the appropriate type for *ptr. in codegenCallMemcpy, update the third argument to have a more useful name and added a comment about its use. In particular, the third argument should only be used when the memcpy is copying exactly one element of that type. Now when emitting a memcpy, only emit tbaa.struct metadata in cases when both it and simple tbaa metadata are available. in codegenCopy, prefer to generate a call to memcpy for cases when we have tbaa.struct metadata already computed. Do it that way because other front-ends (e.g. clang) emit tbaa.struct for memcpy, but we've never seen anything emit tbaa.struct for a load or a store. in the big Expr::codegen, fix two calls to codegenCallMemcpy that were incorrectly passing a third argument, since these calls can be copying != 1 element. Tested with CHPL_LLVM=none make check; tested test/studies and test/release with --llvm, --llvm --llvm-wide-opt, and --llvm --llvm-wide-opt --fast. ------------------------------------------------------------------------------ _______________________________________________ Chapel-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/chapel-developers
