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.
Index: compiler/util/llvmUtil.cpp
===================================================================
--- compiler/util/llvmUtil.cpp (revision 23030)
+++ compiler/util/llvmUtil.cpp (working copy)
@@ -402,7 +402,7 @@
if( ! ty->isSized() ) return false; // who knows how big it is!
uint64_t sz = layout->getTypeSizeInBits(ty);
- sz *= 8; // now in bytes.
+ sz = (sz + 7)/8; // now in bytes.
if( sz < max_size_bytes ) return true;
return false;
Index: compiler/AST/symbol.cpp
===================================================================
--- compiler/AST/symbol.cpp (revision 23030)
+++ compiler/AST/symbol.cpp (working copy)
@@ -1087,20 +1087,31 @@
return;
}
- // Now create tbaa metadata, one for const and one for not.
- {
- llvm::Value* Ops[2];
- Ops[0] = llvm::MDString::get(ctx, cname);
- Ops[1] = parent;
- llvmTbaaNode = llvm::MDNode::get(ctx, Ops);
+ // Only things that aren't 'struct'-like should have simple TBAA
+ // metadata. If they can alias with their fields, we don't do simple TBAA.
+ // Integers, reals, bools, enums, references, wide pointers
+ // count as one thing. Records, strings, complexes should not
+ // get simple TBAA (they can get struct tbaa).
+ if( is_bool_type(type) || is_int_type(type) || is_uint_type(type) ||
+ is_real_type(type) || is_imag_type(type) || is_enum_type(type) ||
+ isClass(type) || hasEitherFlag(FLAG_REF,FLAG_WIDE) ||
+ hasEitherFlag(FLAG_DATA_CLASS,FLAG_WIDE_CLASS) ) {
+ // Now create tbaa metadata, one for const and one for not.
+ {
+ llvm::Value* Ops[2];
+ Ops[0] = llvm::MDString::get(ctx, cname);
+ Ops[1] = parent;
+ llvmTbaaNode = llvm::MDNode::get(ctx, Ops);
+ }
+ {
+ llvm::Value* Ops[3];
+ Ops[0] = llvm::MDString::get(ctx, cname);
+ Ops[1] = constParent;
+ Ops[2] = llvm::ConstantInt::get(llvm::Type::getInt64Ty(ctx), 1);
+ llvmConstTbaaNode = llvm::MDNode::get(ctx, Ops);
+ }
}
- {
- llvm::Value* Ops[3];
- Ops[0] = llvm::MDString::get(ctx, cname);
- Ops[1] = constParent;
- Ops[2] = llvm::ConstantInt::get(llvm::Type::getInt64Ty(ctx), 1);
- llvmConstTbaaNode = llvm::MDNode::get(ctx, Ops);
- }
+
// Don't try to create tbaa.struct metadata for non-struct.
if( isUnion(type) ||
hasFlag(FLAG_STAR_TUPLE) ||
Index: compiler/AST/expr.cpp
===================================================================
--- compiler/AST/expr.cpp (revision 23030)
+++ compiler/AST/expr.cpp (working copy)
@@ -677,7 +677,10 @@
Type* valType = NULL)
{
if( val.chplType && !valType ) valType = val.chplType;
- if( ptr.chplType && !valType ) valType = ptr.chplType->getValType();
+ if( ptr.chplType && !valType ) {
+ if( ptr.isLVPtr ) valType = ptr.chplType;
+ else valType = ptr.chplType->getValType();
+ }
llvm::Type* ptrValType = llvm::cast<llvm::PointerType>(
ptr.val->getType())->getElementType();
@@ -693,7 +696,7 @@
return codegenStoreLLVM(val.val, ptr.val, valType);
}
-// Create an LLVM store instruction possibly adding
+// Create an LLVM load instruction possibly adding
// appropriate metadata based upon the Chapel type of ptr.
static
llvm::LoadInst* codegenLoadLLVM(llvm::Value* ptr,
@@ -716,7 +719,11 @@
Type* valType = NULL,
bool isConst = false)
{
- if( ptr.chplType && !valType ) valType = ptr.chplType->getValType();
+ if( ptr.chplType && !valType ) {
+ if( ptr.isLVPtr ) valType = ptr.chplType;
+ else valType = ptr.chplType->getValType();
+ }
+
return codegenLoadLLVM(ptr.val, valType, isConst);
}
@@ -2660,9 +2667,14 @@
return ret;
}
+// Create a memcpy call copying size bytes from src to dest.
+// If we are copying a single type known to the compiler (e.g. a whole
+// record), pointedToType can contain the single element type. If we
+// are copying (or possibly copying) multiple elements, pointedToType
+// should be NULL. pointedToType is used to emit alias analysis information.
static
void codegenCallMemcpy(GenRet dest, GenRet src, GenRet size,
- Type* eltType) {
+ Type* pointedToType) {
GenInfo *info = gGenInfo;
// Must call with two real pointer arguments
@@ -2711,14 +2723,16 @@
llvm::MDNode* tbaaTag = NULL;
llvm::MDNode* tbaaStructTag = NULL;
- if( eltType ) {
- tbaaTag = eltType->symbol->llvmTbaaNode;
- tbaaStructTag = eltType->symbol->llvmTbaaStructNode;
+ if( pointedToType ) {
+ tbaaTag = pointedToType->symbol->llvmTbaaNode;
+ tbaaStructTag = pointedToType->symbol->llvmTbaaStructNode;
}
- if( tbaaTag )
- CI->setMetadata(llvm::LLVMContext::MD_tbaa, tbaaTag);
+ // For structures, ONLY set the tbaa.struct metadata, since
+ // generally speaking simple tbaa tags don't make sense for structs.
if( tbaaStructTag )
CI->setMetadata(llvm::LLVMContext::MD_tbaa_struct, tbaaStructTag);
+ else if( tbaaTag )
+ CI->setMetadata(llvm::LLVMContext::MD_tbaa, tbaaTag);
#endif
}
}
@@ -2779,7 +2793,13 @@
GenInfo *info = gGenInfo;
if( ! info->cfile ) {
bool useMemcpy = false;
- if( src.isLVPtr ) {
+
+ if( chplType && chplType->symbol->llvmTbaaStructNode ) {
+ // Always use memcpy for things we've developed LLVM
+ // struct nodes for alias analysis, since as far as we know, we
+ // can't use tbaa.struct for load/store.
+ useMemcpy = true;
+ } else if( src.isLVPtr ) {
// Consider using memcpy instead of stack allocating a possibly
// large structure.
@@ -4678,11 +4698,11 @@
if (primitive->tag == PRIM_CHPL_COMM_GET) {
codegenCallMemcpy(localAddr,
codegenAddrOf(codegenWideAddr(lc, remoteAddr)),
- codegenMul(eltSize, len), dt->typeInfo());
+ codegenMul(eltSize, len), NULL);
} else {
codegenCallMemcpy(codegenAddrOf(codegenWideAddr(lc, remoteAddr)),
localAddr,
- codegenMul(eltSize, len), dt->typeInfo());
+ codegenMul(eltSize, len), NULL);
}
}
break;
------------------------------------------------------------------------------
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
