Skip to content

[SCCP] Infer return attributes in SCCP as well #106732

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
merged 3 commits into from
Sep 2, 2024
Merged

[SCCP] Infer return attributes in SCCP as well #106732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50efbbd
[SCCP] Infer return attributes in SCCP as well
nikic Aug 30, 2024
9eaece6
Update clang test
nikic Aug 30, 2024
5e3508f
Apply suggestions from code review
nikic Sep 2, 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
8 changes: 4 additions & 4 deletions clang/test/CodeGen/attr-counted-by.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ void test6(struct anon_struct *p, int index) {
p->array[index] = __builtin_dynamic_object_size(p->array, 1);
}

// SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @test6_bdos(
// SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 0, -3) i64 @test6_bdos(
// SANITIZE-WITH-ATTR-SAME: ptr nocapture noundef readonly [[P:%.*]]) local_unnamed_addr #[[ATTR2]] {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 8
Expand All @@ -649,7 +649,7 @@ void test6(struct anon_struct *p, int index) {
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = select i1 [[DOTINV]], i64 0, i64 [[TMP0]]
// SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP1]]
//
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @test6_bdos(
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 0, -3) i64 @test6_bdos(
// NO-SANITIZE-WITH-ATTR-SAME: ptr nocapture noundef readonly [[P:%.*]]) local_unnamed_addr #[[ATTR2]] {
// NO-SANITIZE-WITH-ATTR-NEXT: entry:
// NO-SANITIZE-WITH-ATTR-NEXT: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 8
Expand Down Expand Up @@ -955,7 +955,7 @@ void test10(struct union_of_fams *p, int index) {
p->bytes[index] = (unsigned char)__builtin_dynamic_object_size(p->bytes, 1);
}

// SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -2147483648, 2147483648) i64 @test10_bdos(
// SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 0, 2147483648) i64 @test10_bdos(
// SANITIZE-WITH-ATTR-SAME: ptr nocapture noundef readonly [[P:%.*]]) local_unnamed_addr #[[ATTR2]] {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 8
Expand All @@ -964,7 +964,7 @@ void test10(struct union_of_fams *p, int index) {
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = zext nneg i32 [[NARROW]] to i64
// SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP0]]
//
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -2147483648, 2147483648) i64 @test10_bdos(
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 0, 2147483648) i64 @test10_bdos(
// NO-SANITIZE-WITH-ATTR-SAME: ptr nocapture noundef readonly [[P:%.*]]) local_unnamed_addr #[[ATTR2]] {
// NO-SANITIZE-WITH-ATTR-NEXT: entry:
// NO-SANITIZE-WITH-ATTR-NEXT: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds i8, ptr [[P]], i64 8
Expand Down
4 changes: 3 additions & 1 deletion llvm/include/llvm/Transforms/Utils/SCCPSolver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class SCCPSolver {
const ValueLatticeElement &getLatticeValueFor(Value *V) const;

/// getTrackedRetVals - Get the inferred return value map.
const MapVector<Function *, ValueLatticeElement> &getTrackedRetVals();
const MapVector<Function *, ValueLatticeElement> &getTrackedRetVals() const;

/// getTrackedGlobals - Get and return the set of inferred initializers for
/// global variables.
Expand Down Expand Up @@ -190,6 +190,8 @@ class SCCPSolver {
bool removeNonFeasibleEdges(BasicBlock *BB, DomTreeUpdater &DTU,
BasicBlock *&NewUnreachableBB) const;

void inferReturnAttributes() const;

bool tryToReplaceWithConstant(Value *V);

// Helper to check if \p LV is either a constant or a constant
Expand Down
32 changes: 4 additions & 28 deletions llvm/lib/Transforms/IPO/SCCP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,34 +277,10 @@ static bool runIPSCCP(
// whether other functions are optimizable.
SmallVector<ReturnInst*, 8> ReturnsToZap;

for (const auto &I : Solver.getTrackedRetVals()) {
Function *F = I.first;
const ValueLatticeElement &ReturnValue = I.second;

// If there is a known constant range for the return value, add range
// attribute to the return value.
if (ReturnValue.isConstantRange() &&
!ReturnValue.getConstantRange().isSingleElement()) {
// Do not add range metadata if the return value may include undef.
if (ReturnValue.isConstantRangeIncludingUndef())
continue;

// Take the intersection of the existing attribute and the inferred range.
ConstantRange CR = ReturnValue.getConstantRange();
if (F->hasRetAttribute(Attribute::Range))
CR = CR.intersectWith(F->getRetAttribute(Attribute::Range).getRange());
F->addRangeRetAttr(CR);
continue;
}
// Infer nonnull return attribute.
if (F->getReturnType()->isPointerTy() && ReturnValue.isNotConstant() &&
ReturnValue.getNotConstant()->isNullValue() &&
!F->hasRetAttribute(Attribute::NonNull)) {
F->addRetAttr(Attribute::NonNull);
continue;
}
if (F->getReturnType()->isVoidTy())
continue;
Solver.inferReturnAttributes();
for (const auto &[F, ReturnValue] : Solver.getTrackedRetVals()) {
assert(!F->getReturnType()->isVoidTy() &&
"should not track void functions");
if (SCCPSolver::isConstant(ReturnValue) || ReturnValue.isUnknownOrUndef())
findReturnsToZap(*F, ReturnsToZap, Solver);
}
Expand Down
8 changes: 8 additions & 0 deletions llvm/lib/Transforms/Scalar/SCCP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/Analysis/ValueLatticeUtils.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/Constant.h"
Expand Down Expand Up @@ -66,6 +67,11 @@ static bool runSCCP(Function &F, const DataLayout &DL,
DL, [TLI](Function &F) -> const TargetLibraryInfo & { return *TLI; },
F.getContext());

// While we don't do any actual inter-procedural analysis, still track
// return values so we can infer attributes.
if (canTrackReturnsInterprocedurally(&F))
Solver.addTrackedFunction(&F);

// Mark the first block of the function as being executable.
Solver.markBlockExecutable(&F.front());

Expand Down Expand Up @@ -115,6 +121,8 @@ static bool runSCCP(Function &F, const DataLayout &DL,
if (!DeadBB->hasAddressTaken())
DTU.deleteBB(DeadBB);

Solver.inferReturnAttributes();

return MadeChanges;
}

Expand Down
30 changes: 29 additions & 1 deletion llvm/lib/Transforms/Utils/SCCPSolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,34 @@ bool SCCPSolver::removeNonFeasibleEdges(BasicBlock *BB, DomTreeUpdater &DTU,
return true;
}

void SCCPSolver::inferReturnAttributes() const {
for (const auto &[F, ReturnValue] : getTrackedRetVals()) {

// If there is a known constant range for the return value, add range
// attribute to the return value.
if (ReturnValue.isConstantRange() &&
!ReturnValue.getConstantRange().isSingleElement()) {
// Do not add range metadata if the return value may include undef.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any tests for this.

Is the reason for this to avoid creating UB where we where just propagating poison beforehand? If so, do you also need noundef for propagating nonnull?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any tests for this.

This is pre-existing code moved to a different place.

Is the reason for this to avoid creating UB where we where just propagating poison beforehand? If so, do you also need noundef for propagating nonnull?

ConstantRangeIncludingUndef means ConstantRange|undef. In the undef case any value may be picked, so the result is a full range.

if (ReturnValue.isConstantRangeIncludingUndef())
continue;

// Take the intersection of the existing attribute and the inferred range.
ConstantRange CR = ReturnValue.getConstantRange();
if (F->hasRetAttribute(Attribute::Range))
CR = CR.intersectWith(F->getRetAttribute(Attribute::Range).getRange());
F->addRangeRetAttr(CR);
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we ever want to infer nonnull in addition to range attr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nonnull applies only to pointers, while range applies only to (vectors of) integers, so they can not apply at the same time.

}
// Infer nonnull return attribute.
if (F->getReturnType()->isPointerTy() && ReturnValue.isNotConstant() &&
ReturnValue.getNotConstant()->isNullValue() &&
!F->hasRetAttribute(Attribute::NonNull)) {
F->addRetAttr(Attribute::NonNull);
continue;
}
}
}

/// Helper class for SCCPSolver. This implements the instruction visitor and
/// holds all the state.
class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> {
Expand Down Expand Up @@ -2168,7 +2196,7 @@ const ValueLatticeElement &SCCPSolver::getLatticeValueFor(Value *V) const {
}

const MapVector<Function *, ValueLatticeElement> &
SCCPSolver::getTrackedRetVals() {
SCCPSolver::getTrackedRetVals() const {
return Visitor->getTrackedRetVals();
}

Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/PhaseOrdering/icmp-ashr-breaking-select-idiom.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
; RUN: opt -O1 -S < %s | FileCheck %s

define i32 @testa(i32 %mul) {
; CHECK-LABEL: define range(i32 -65536, 65536) i32 @testa(
; CHECK-LABEL: define range(i32 -65536, 32768) i32 @testa(
; CHECK-SAME: i32 [[MUL:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: [[SHR:%.*]] = ashr i32 [[MUL]], 15
; CHECK-NEXT: [[SPEC_SELECT_I:%.*]] = tail call i32 @llvm.smin.i32(i32 [[SHR]], i32 32767)
Expand All @@ -16,7 +16,7 @@ define i32 @testa(i32 %mul) {
}

define i32 @testb(i32 %mul) {
; CHECK-LABEL: define range(i32 -16777216, 16777216) i32 @testb(
; CHECK-LABEL: define range(i32 -128, 128) i32 @testb(
; CHECK-SAME: i32 [[MUL:%.*]]) local_unnamed_addr #[[ATTR0]] {
; CHECK-NEXT: [[SHR102:%.*]] = ashr i32 [[MUL]], 7
; CHECK-NEXT: [[TMP1:%.*]] = tail call i32 @llvm.smax.i32(i32 [[SHR102]], i32 -128)
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/SCCP/exact-flags.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
; RUN: opt -passes=sccp < %s -S | FileCheck %s

define i8 @ashr_to_lshr(i8 %x, i8 %y) {
; CHECK-LABEL: define i8 @ashr_to_lshr(
; CHECK-LABEL: define range(i8 0, -128) i8 @ashr_to_lshr(
; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
; CHECK-NEXT: [[P:%.*]] = and i8 [[X]], 127
; CHECK-NEXT: [[R:%.*]] = lshr exact i8 [[P]], [[Y]]
Expand All @@ -14,7 +14,7 @@ define i8 @ashr_to_lshr(i8 %x, i8 %y) {
}

define i8 @sdiv_to_udiv(i8 %x, i8 %y) {
; CHECK-LABEL: define i8 @sdiv_to_udiv(
; CHECK-LABEL: define range(i8 0, -128) i8 @sdiv_to_udiv(
; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
; CHECK-NEXT: [[X1:%.*]] = and i8 [[X]], 127
; CHECK-NEXT: [[Y1:%.*]] = and i8 [[Y]], 127
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/SCCP/phis.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ end:
}

define <2 x i16> @phi_vector_merge1(i1 %c, <2 x i8> %a) {
; CHECK-LABEL: define <2 x i16> @phi_vector_merge1(
; CHECK-LABEL: define range(i16 2, 259) <2 x i16> @phi_vector_merge1(
; CHECK-SAME: i1 [[C:%.*]], <2 x i8> [[A:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: [[ZEXT:%.*]] = zext <2 x i8> [[A]] to <2 x i16>
Expand All @@ -126,7 +126,7 @@ join:
}

define <2 x i16> @phi_vector_merge2(i1 %c, <2 x i8> %a) {
; CHECK-LABEL: define <2 x i16> @phi_vector_merge2(
; CHECK-LABEL: define range(i16 2, 259) <2 x i16> @phi_vector_merge2(
; CHECK-SAME: i1 [[C:%.*]], <2 x i8> [[A:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: [[ZEXT:%.*]] = zext <2 x i8> [[A]] to <2 x i16>
Expand Down
10 changes: 3 additions & 7 deletions llvm/test/Transforms/SCCP/pointer-nonnull.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,9 @@ define i1 @ip_test_nonnull_caller(ptr %p) {
}

define ptr @ret_nonnull_pointer(ptr nonnull %p) {
; SCCP-LABEL: define ptr @ret_nonnull_pointer(
; SCCP-SAME: ptr nonnull [[P:%.*]]) {
; SCCP-NEXT: ret ptr [[P]]
;
; IPSCCP-LABEL: define nonnull ptr @ret_nonnull_pointer(
; IPSCCP-SAME: ptr nonnull [[P:%.*]]) {
; IPSCCP-NEXT: ret ptr [[P]]
; CHECK-LABEL: define nonnull ptr @ret_nonnull_pointer(
; CHECK-SAME: ptr nonnull [[P:%.*]]) {
; CHECK-NEXT: ret ptr [[P]]
;
ret ptr %p
}
Expand Down
Loading