[llvm] r317311 - [Analysis] Refine matching and merging of TBAA tags
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 7 01:10:31 PST 2017
Hi Ivan,
I've run into a problem when using your patch, could you please take a
quick look at this and see if this is expected?
The problem that I get is that a load and a store are rescheduled so the
load is done before the store, but it should be after.
Looking at what happens, I see that with your patch I get a difference
in output from the load store vectorizer. The tbaa-metadata, !21, for
the merged store is slightly different.
Before your change I get
!12 = !{!"omnipotent char", !13, i64 0}
!13 = !{!"Simple C/C++ TBAA"}
!21 = !{!12, !12, i64 0}
and with the patch I instead get
!12 = !{!"omnipotent char", !13, i64 0}
!13 = !{!"Simple C/C++ TBAA"}
!14 = !{!"short", !12, i64 0}
!21 = !{!14, !14, i64 0}
The two merged stores are a char and a short. On my target, both of them
are i16 (we have a huge patch making llvm accept 16b bytes).
So, is the above difference in output intended? Is it correct?
I attached log printouts from LSV with before and after dumps. The whole
module is dumped so metadata is shown as well.
Regards,
Mikael
On 11/03/2017 11:26 AM, Ivan A. Kosarev via llvm-commits wrote:
> Author: kosarev
> Date: Fri Nov 3 03:26:25 2017
> New Revision: 317311
>
> URL: http://llvm.org/viewvc/llvm-project?rev=317311&view=rev
> Log:
> [Analysis] Refine matching and merging of TBAA tags
>
> This patch combines the code that matches and merges TBAA access
> tags. The aim is to simplify future changes and making sure that
> these operations produce consistent results.
>
> Differential Revision: https://reviews.llvm.org/D39463
>
> Modified:
> llvm/trunk/lib/Analysis/TypeBasedAliasAnalysis.cpp
>
> Modified: llvm/trunk/lib/Analysis/TypeBasedAliasAnalysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/TypeBasedAliasAnalysis.cpp?rev=317311&r1=317310&r2=317311&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/TypeBasedAliasAnalysis.cpp (original)
> +++ llvm/trunk/lib/Analysis/TypeBasedAliasAnalysis.cpp Fri Nov 3 03:26:25 2017
> @@ -314,17 +314,8 @@ AliasResult TypeBasedAAResult::alias(con
> if (!EnableTBAA)
> return AAResultBase::alias(LocA, LocB);
>
> - // Get the attached MDNodes. If either value lacks a tbaa MDNode, we must
> - // be conservative.
> - const MDNode *AM = LocA.AATags.TBAA;
> - if (!AM)
> - return AAResultBase::alias(LocA, LocB);
> - const MDNode *BM = LocB.AATags.TBAA;
> - if (!BM)
> - return AAResultBase::alias(LocA, LocB);
> -
> - // If they may alias, chain to the next AliasAnalysis.
> - if (Aliases(AM, BM))
> + // If accesses may alias, chain to the next AliasAnalysis.
> + if (Aliases(LocA.AATags.TBAA, LocB.AATags.TBAA))
> return AAResultBase::alias(LocA, LocB);
>
> // Otherwise return a definitive result.
> @@ -424,25 +415,24 @@ bool MDNode::isTBAAVtableAccess() const
> return false;
> }
>
> +static bool matchAccessTags(const MDNode *A, const MDNode *B,
> + const MDNode **GenericTag = nullptr);
> +
> MDNode *MDNode::getMostGenericTBAA(MDNode *A, MDNode *B) {
> + const MDNode *GenericTag;
> + matchAccessTags(A, B, &GenericTag);
> + return const_cast<MDNode*>(GenericTag);
> +}
> +
> +static const MDNode *getLeastCommonType(const MDNode *A, const MDNode *B) {
> if (!A || !B)
> return nullptr;
>
> if (A == B)
> return A;
>
> - // For struct-path aware TBAA, we use the access type of the tag.
> - assert(isStructPathTBAA(A) && isStructPathTBAA(B) &&
> - "Auto upgrade should have taken care of this!");
> - A = cast_or_null<MDNode>(MutableTBAAStructTagNode(A).getAccessType());
> - if (!A)
> - return nullptr;
> - B = cast_or_null<MDNode>(MutableTBAAStructTagNode(B).getAccessType());
> - if (!B)
> - return nullptr;
> -
> - SmallSetVector<MDNode *, 4> PathA;
> - MutableTBAANode TA(A);
> + SmallSetVector<const MDNode *, 4> PathA;
> + TBAANode TA(A);
> while (TA.getNode()) {
> if (PathA.count(TA.getNode()))
> report_fatal_error("Cycle found in TBAA metadata.");
> @@ -450,8 +440,8 @@ MDNode *MDNode::getMostGenericTBAA(MDNod
> TA = TA.getParent();
> }
>
> - SmallSetVector<MDNode *, 4> PathB;
> - MutableTBAANode TB(B);
> + SmallSetVector<const MDNode *, 4> PathB;
> + TBAANode TB(B);
> while (TB.getNode()) {
> if (PathB.count(TB.getNode()))
> report_fatal_error("Cycle found in TBAA metadata.");
> @@ -462,7 +452,7 @@ MDNode *MDNode::getMostGenericTBAA(MDNod
> int IA = PathA.size() - 1;
> int IB = PathB.size() - 1;
>
> - MDNode *Ret = nullptr;
> + const MDNode *Ret = nullptr;
> while (IA >= 0 && IB >= 0) {
> if (PathA[IA] == PathB[IB])
> Ret = PathA[IA];
> @@ -472,17 +462,7 @@ MDNode *MDNode::getMostGenericTBAA(MDNod
> --IB;
> }
>
> - // We either did not find a match, or the only common base "type" is
> - // the root node. In either case, we don't have any useful TBAA
> - // metadata to attach.
> - if (!Ret || Ret->getNumOperands() < 2)
> - return nullptr;
> -
> - // We need to convert from a type node to a tag node.
> - Type *Int64 = IntegerType::get(A->getContext(), 64);
> - Metadata *Ops[3] = {Ret, Ret,
> - ConstantAsMetadata::get(ConstantInt::get(Int64, 0))};
> - return MDNode::get(A->getContext(), Ops);
> + return Ret;
> }
>
> void Instruction::getAAMetadata(AAMDNodes &N, bool Merge) const {
> @@ -505,70 +485,107 @@ void Instruction::getAAMetadata(AAMDNode
> N.NoAlias = getMetadata(LLVMContext::MD_noalias);
> }
>
> -/// Aliases - Test whether the type represented by A may alias the
> -/// type represented by B.
> -bool TypeBasedAAResult::Aliases(const MDNode *A, const MDNode *B) const {
> +static bool findAccessType(TBAAStructTagNode BaseTag,
> + const MDNode *AccessTypeNode,
> + uint64_t &OffsetInBase) {
> + // Start from the base type, follow the edge with the correct offset in
> + // the type DAG and adjust the offset until we reach the access type or
> + // until we reach a root node.
> + TBAAStructTypeNode BaseType(BaseTag.getBaseType());
> + OffsetInBase = BaseTag.getOffset();
> +
> + while (const MDNode *BaseTypeNode = BaseType.getNode()) {
> + if (BaseTypeNode == AccessTypeNode)
> + return true;
> +
> + // Follow the edge with the correct offset, Offset will be adjusted to
> + // be relative to the field type.
> + BaseType = BaseType.getParent(OffsetInBase);
> + }
> + return false;
> +}
> +
> +static const MDNode *createAccessTag(const MDNode *AccessType) {
> + Type *Int64 = IntegerType::get(AccessType->getContext(), 64);
> + auto *ImmutabilityFlag = ConstantAsMetadata::get(ConstantInt::get(Int64, 0));
> + Metadata *Ops[] = {const_cast<MDNode*>(AccessType),
> + const_cast<MDNode*>(AccessType), ImmutabilityFlag};
> + return MDNode::get(AccessType->getContext(), Ops);
> +}
> +
> +/// matchTags - Return true if the given couple of accesses are allowed to
> +/// overlap. If \arg GenericTag is not null, then on return it points to the
> +/// most generic access descriptor for the given two.
> +static bool matchAccessTags(const MDNode *A, const MDNode *B,
> + const MDNode **GenericTag) {
> + if (A == B) {
> + if (GenericTag)
> + *GenericTag = A;
> + return true;
> + }
> +
> + // Accesses with no TBAA information may alias with any other accesses.
> + if (!A || !B) {
> + if (GenericTag)
> + *GenericTag = nullptr;
> + return true;
> + }
> +
> // Verify that both input nodes are struct-path aware. Auto-upgrade should
> // have taken care of this.
> - assert(isStructPathTBAA(A) && "MDNode A is not struct-path aware.");
> - assert(isStructPathTBAA(B) && "MDNode B is not struct-path aware.");
> + assert(isStructPathTBAA(A) && "Access A is not struct-path aware!");
> + assert(isStructPathTBAA(B) && "Access B is not struct-path aware!");
>
> - // Keep track of the root node for A and B.
> - TBAAStructTypeNode RootA, RootB;
> TBAAStructTagNode TagA(A), TagB(B);
>
> // TODO: We need to check if AccessType of TagA encloses AccessType of
> // TagB to support aggregate AccessType. If yes, return true.
>
> - // Start from the base type of A, follow the edge with the correct offset in
> - // the type DAG and adjust the offset until we reach the base type of B or
> - // until we reach the Root node.
> - // Compare the adjusted offset once we have the same base.
> -
> - // Climb the type DAG from base type of A to see if we reach base type of B.
> const MDNode *BaseA = TagA.getBaseType();
> const MDNode *BaseB = TagB.getBaseType();
> - uint64_t OffsetA = TagA.getOffset(), OffsetB = TagB.getOffset();
> - for (TBAAStructTypeNode T(BaseA);;) {
> - if (T.getNode() == BaseB)
> - // Base type of A encloses base type of B, check if the offsets match.
> - return OffsetA == OffsetB;
>
> - RootA = T;
> - // Follow the edge with the correct offset, OffsetA will be adjusted to
> - // be relative to the field type.
> - T = T.getParent(OffsetA);
> - if (!T.getNode())
> - break;
> + // Climb the type DAG from base type of A to see if we reach base type of B.
> + uint64_t OffsetA;
> + if (findAccessType(TagA, BaseB, OffsetA)) {
> + if (GenericTag)
> + *GenericTag = createAccessTag(TagB.getAccessType());
> + return OffsetA == TagB.getOffset();
> }
>
> - // Reset OffsetA and climb the type DAG from base type of B to see if we reach
> - // base type of A.
> - OffsetA = TagA.getOffset();
> - for (TBAAStructTypeNode T(BaseB);;) {
> - if (T.getNode() == BaseA)
> - // Base type of B encloses base type of A, check if the offsets match.
> - return OffsetA == OffsetB;
> -
> - RootB = T;
> - // Follow the edge with the correct offset, OffsetB will be adjusted to
> - // be relative to the field type.
> - T = T.getParent(OffsetB);
> - if (!T.getNode())
> - break;
> + // Climb the type DAG from base type of B to see if we reach base type of A.
> + uint64_t OffsetB;
> + if (findAccessType(TagB, BaseA, OffsetB)) {
> + if (GenericTag)
> + *GenericTag = createAccessTag(TagA.getAccessType());
> + return OffsetB == TagA.getOffset();
> }
>
> - // Neither node is an ancestor of the other.
> + // If neither node is an ancestor of the other, then try to find the type
> + // that is common to both the final access types.
> + const MDNode *CommonType = getLeastCommonType(TagA.getAccessType(),
> + TagB.getAccessType());
> +
> + // If there is no common type or the only common type is the root node, then
> + // we don't have any useful generic access tag to return.
> + if (GenericTag)
> + *GenericTag = !CommonType || CommonType->getNumOperands() < 2 ?
> + nullptr : createAccessTag(CommonType);
>
> // If they have different roots, they're part of different potentially
> // unrelated type systems, so we must be conservative.
> - if (RootA.getNode() != RootB.getNode())
> + if (!CommonType)
> return true;
>
> // If they have the same root, then we've proved there's no alias.
> return false;
> }
>
> +/// Aliases - Test whether the access represented by tag A may alias the
> +/// access represented by tag B.
> +bool TypeBasedAAResult::Aliases(const MDNode *A, const MDNode *B) const {
> + return matchAccessTags(A, B);
> +}
> +
> AnalysisKey TypeBasedAA::Key;
>
> TypeBasedAAResult TypeBasedAA::run(Function &F, FunctionAnalysisManager &AM) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lsv.log
Type: text/x-log
Size: 10850 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171107/daa850b5/attachment.bin>
More information about the llvm-commits
mailing list