Skip to content

Commit 65fd9f1

Browse files
authored
[Attributor] Support nested conditional branches (#168532)
The attributor can infer the alignment of %p at the call-site in this example [1]: ``` define void @f(ptr align 8 %p, i1 %c1, i1 %c2) { entry: br i1 %c1, label %bb.1, label %exit bb.1: call void (...) @llvm.fake.use(ptr %p) br label %exit exit: ret void } ``` but not when there's an additional conditional branch: ``` define void @f(ptr align 8 %p, i1 %c1, i1 %c2) { entry: br i1 %c1, label %bb.1, label %exit bb.1: br i1 %c2, label %bb.2, label %exit bb.2: call void (...) @llvm.fake.use(ptr %p) br label %exit exit: ret void } ``` unless `-attributor-annotate-decl-cs` is enabled. This patch extends `followUsesInMBEC` to handle such recursive branches. n.b. admittedly I wrote this patch before discovering inferring the alignment in this example is already possible with `-attributor-annotate-decl-cs`, I came to realise this once writing the tests, but this seems like a gap regardless looking at existing FIXMEs, plus the alignment can now be inferred in this particular example without the flag. [1] https://godbolt.org/z/aKoc75so5
1 parent e4cff3c commit 65fd9f1

File tree

6 files changed

+234
-229
lines changed

6 files changed

+234
-229
lines changed

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,10 @@ static void followUsesInMBEC(AAType &AA, Attributor &A, StateType &S,
665665
return;
666666

667667
SmallVector<const BranchInst *, 4> BrInsts;
668+
SmallPtrSet<const Instruction *, 16> Visited;
668669
auto Pred = [&](const Instruction *I) {
670+
if (!Visited.insert(I).second)
671+
return false;
669672
if (const BranchInst *Br = dyn_cast<BranchInst>(I))
670673
if (Br->isConditional())
671674
BrInsts.push_back(Br);
@@ -684,28 +687,10 @@ static void followUsesInMBEC(AAType &AA, Attributor &A, StateType &S,
684687
// ParentS_m = ChildS_{m, 1} /\ ChildS_{m, 2} /\ ... /\ ChildS_{m, n_m}
685688
//
686689
// Known State |= ParentS_1 \/ ParentS_2 \/... \/ ParentS_m
687-
//
688-
// FIXME: Currently, recursive branches are not handled. For example, we
689-
// can't deduce that ptr must be dereferenced in below function.
690-
//
691-
// void f(int a, int c, int *ptr) {
692-
// if(a)
693-
// if (b) {
694-
// *ptr = 0;
695-
// } else {
696-
// *ptr = 1;
697-
// }
698-
// else {
699-
// if (b) {
700-
// *ptr = 0;
701-
// } else {
702-
// *ptr = 1;
703-
// }
704-
// }
705-
// }
706690

707691
Explorer->checkForAllContext(&CtxI, Pred);
708-
for (const BranchInst *Br : BrInsts) {
692+
while (!BrInsts.empty()) {
693+
const BranchInst *Br = BrInsts.pop_back_val();
709694
StateType ParentState;
710695

711696
// The known state of the parent state is a conjunction of children's
@@ -714,15 +699,18 @@ static void followUsesInMBEC(AAType &AA, Attributor &A, StateType &S,
714699

715700
for (const BasicBlock *BB : Br->successors()) {
716701
StateType ChildState;
717-
718702
size_t BeforeSize = Uses.size();
719-
followUsesInContext(AA, A, *Explorer, &BB->front(), Uses, ChildState);
703+
const Instruction *I = &BB->front();
704+
followUsesInContext(AA, A, *Explorer, I, Uses, ChildState);
720705

721706
// Erase uses which only appear in the child.
722707
for (auto It = Uses.begin() + BeforeSize; It != Uses.end();)
723708
It = Uses.erase(It);
724709

725710
ParentState &= ChildState;
711+
712+
// Check for recursive conditional branches.
713+
Explorer->checkForAllContext(I, Pred);
726714
}
727715

728716
// Use only known state.

llvm/test/Transforms/Attributor/dereferenceable-1.ll

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -555,12 +555,10 @@ cont2:
555555
; *ptr = 4;
556556
; }
557557
; }
558-
;
559-
; FIXME: %ptr should be dereferenceable(4)
560558
define dso_local void @rec-branch-1(i32 %a, i32 %b, i32 %c, ptr %ptr) {
561559
; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: write)
562560
; CHECK-LABEL: define {{[^@]+}}@rec-branch-1
563-
; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i32 [[C:%.*]], ptr nofree writeonly captures(none) [[PTR:%.*]]) #[[ATTR3]] {
561+
; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i32 [[C:%.*]], ptr nofree nonnull writeonly align 4 captures(none) dereferenceable(4) [[PTR:%.*]]) #[[ATTR3]] {
564562
; CHECK-NEXT: entry:
565563
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[A]], 0
566564
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_ELSE3:%.*]], label [[IF_THEN:%.*]]
@@ -630,11 +628,10 @@ if.end8: ; preds = %if.then5, %if.else6
630628
; rec-branch-2(1, 1, 1, ptr);
631629
; }
632630
; }
633-
; FIXME: %ptr should be dereferenceable(4)
634631
define dso_local void @rec-branch-2(i32 %a, i32 %b, i32 %c, ptr %ptr) {
635632
; CHECK: Function Attrs: nofree nosync nounwind memory(argmem: write)
636633
; CHECK-LABEL: define {{[^@]+}}@rec-branch-2
637-
; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i32 [[C:%.*]], ptr nofree writeonly captures(none) [[PTR:%.*]]) #[[ATTR5:[0-9]+]] {
634+
; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i32 [[C:%.*]], ptr nofree nonnull writeonly align 4 captures(none) dereferenceable(4) [[PTR:%.*]]) #[[ATTR5:[0-9]+]] {
638635
; CHECK-NEXT: entry:
639636
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[A]], 0
640637
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_ELSE3:%.*]], label [[IF_THEN:%.*]]
@@ -654,7 +651,7 @@ define dso_local void @rec-branch-2(i32 %a, i32 %b, i32 %c, ptr %ptr) {
654651
; CHECK-NEXT: store i32 3, ptr [[PTR]], align 4
655652
; CHECK-NEXT: br label [[IF_END8]]
656653
; CHECK: if.else6:
657-
; CHECK-NEXT: tail call void @rec-branch-2(i32 noundef 1, i32 noundef 1, i32 noundef 1, ptr nofree writeonly captures(none) [[PTR]]) #[[ATTR8:[0-9]+]]
654+
; CHECK-NEXT: tail call void @rec-branch-2(i32 noundef 1, i32 noundef 1, i32 noundef 1, ptr nofree nonnull writeonly align 4 captures(none) dereferenceable(4) [[PTR]]) #[[ATTR8:[0-9]+]]
658655
; CHECK-NEXT: br label [[IF_END8]]
659656
; CHECK: if.end8:
660657
; CHECK-NEXT: ret void

0 commit comments

Comments
 (0)