Skip to content

[TBAA] Fix the case where a subobject gets accessed at a non-zero offset. #101485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kosarev merged 2 commits into from
Sep 6, 2024
Merged

[TBAA] Fix the case where a subobject gets accessed at a non-zero offset. #101485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38feab9
[TBAA] Fix the case where a subobject gets accessed at a non-zero off…
kosarev Aug 1, 2024
32e176f
Merge branch 'main:439d7de14d71' into tbaa-fix
kosarev Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,12 +613,13 @@ static bool mayBeAccessToSubobjectOf(TBAAStructTagNode BaseTag,
}

if (BaseType.getNode() == SubobjectTag.getBaseType()) {
Copy link
Collaborator

@efriedma-quic efriedma-quic Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I can't follow the logic here at all. What is the purpose of the code inside this if statement?

Copy link
Collaborator Author

@kosarev kosarev Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the type of the current base type matched the type of the subobject type, we see if the accesses within these same-typed objects may alias. So far we were just looking at the offsets, which doesn't work when one of the accesses dereferences the whole object, meaning offset 0, and the other dereferences its field/element at offset other than 0. The change fixes that by catching the case where any of the accesses are whole-object ones.

Copy link
Collaborator

@efriedma-quic efriedma-quic Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I see... the old code detects whether both accesses refer to the same object, but we also care about overlapping objects. According to the TBAA rules, objects have to be properly nested. So the only way to get overlap is if one of the accesses is a "whole-object" access. Therefore, this check.

LangRef says "the access type is a scalar type descriptor", which I guess isn't accurate? Please propose a patch to fix the language there.

Copy link
Collaborator Author

@kosarev kosarev Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LangRef doesn't cover the new aggregate-enabled TBAA format, which this patch is about. The latest plan documentation-wise I'm aware of is to wait until the new format is mature enough to be enabled by default, and then update the documentation, see https://reviews.llvm.org/D40975#955973. Until then it's all work-in-progress and not something supposed to be exposed to the user.

bool SameMemberAccess = OffsetInBase == SubobjectTag.getOffset();
MayAlias = OffsetInBase == SubobjectTag.getOffset() ||
BaseType.getNode() == BaseTag.getAccessType() ||
SubobjectTag.getBaseType() == SubobjectTag.getAccessType();
if (GenericTag) {
*GenericTag = SameMemberAccess ? SubobjectTag.getNode() :
createAccessTag(CommonType);
*GenericTag =
MayAlias ? SubobjectTag.getNode() : createAccessTag(CommonType);
}
MayAlias = SameMemberAccess;
return true;
}

Expand Down
23 changes: 21 additions & 2 deletions llvm/test/Analysis/TypeBasedAliasAnalysis/aggregates.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
; Check that TBAA handles access tags with aggregate final access types
; correctly.

%A = type { i32 } ; struct A { int i; };
%A = type { i32, i32 } ; struct A { int i, j; };
%B = type { %A } ; struct B { A a; };
%C = type { %B } ; struct C { B b; };
%D = type { i16 } ; struct D { short s; };
Expand Down Expand Up @@ -105,13 +105,32 @@ entry:
ret i32 %0
}

; A vs. A::j => MayAlias.
; This differs from A vs. A::i case in that the offsets of the final
; accessed objects in A do not match.
define i32 @f7(ptr %i, ptr %a) {
entry:
; CHECK-LABEL: f7
; CHECK: MayAlias: store i32 7, {{.*}} <-> store i32 5,
; OPT-LABEL: f7
; OPT: store i32 5,
; OPT: store i32 7,
; OPT: %[[RET:.*]] = load i32,
; OPT: ret i32 %[[RET]]
store i32 5, ptr %i, align 4, !tbaa !10 ; TAG_A
store i32 7, ptr %a, align 4, !tbaa !16 ; TAG_A_j
%0 = load i32, ptr %i, align 4, !tbaa !10 ; TAG_A
ret i32 %0
}

!0 = !{!"root"}
!1 = !{!0, i64 1, !"char"}
!2 = !{!1, i64 4, !"int"}
!3 = !{!2, !2, i64 0, i64 4} ; TAG_int

!4 = !{!1, i64 4, !"A", !2, i64 0, i64 4}
!4 = !{!1, i64 4, !"A", !2, i64 0, i64 4, !2, i64 4, i64 4}
!5 = !{!4, !2, i64 0, i64 4} ; TAG_A_i
!16 = !{!4, !2, i64 4, i64 4} ; TAG_A_j

!6 = !{!1, i64 4, !"B", !4, i64 0, i64 4}
!7 = !{!6, !4, i64 0, i64 4} ; TAG_B_a
Expand Down