Skip to content

[InstCombine] Remove redundant alignment assumptions. #123348

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[InstCombine] Remove redundant alignment assumptions. #123348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e683b8e
[InstCombine] Remove redundant alignment assumptions.
fhahn Jan 18, 2025
86c5a81
!fixup skip for args, no context instr.
fhahn Jan 21, 2025
32769c7
!fixup avoid overflow
fhahn Jan 21, 2025
36756d6
!fixup fix Linux build failure
fhahn Jan 21, 2025
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
33 changes: 29 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3226,12 +3226,13 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// TODO: apply range metadata for range check patterns?
}

// Separate storage assumptions apply to the underlying allocations, not any
// particular pointer within them. When evaluating the hints for AA purposes
// we getUnderlyingObject them; by precomputing the answers here we can
// avoid having to do so repeatedly there.
for (unsigned Idx = 0; Idx < II->getNumOperandBundles(); Idx++) {
OperandBundleUse OBU = II->getOperandBundleAt(Idx);

// Separate storage assumptions apply to the underlying allocations, not
// any particular pointer within them. When evaluating the hints for AA
// purposes we getUnderlyingObject them; by precomputing the answers here
// we can avoid having to do so repeatedly there.
if (OBU.getTagName() == "separate_storage") {
assert(OBU.Inputs.size() == 2);
auto MaybeSimplifyHint = [&](const Use &U) {
Expand All @@ -3245,6 +3246,30 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
MaybeSimplifyHint(OBU.Inputs[0]);
MaybeSimplifyHint(OBU.Inputs[1]);
}

// Try to fold alignment assumption into a load's !align metadata, if the
// assumption is valid in the load's context and remove redundant ones.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused -- the code doesn't seem to do anything with load metadata?

if (OBU.getTagName() == "align" && OBU.Inputs.size() == 2) {
RetainedKnowledge RK = getKnowledgeFromBundle(
*cast<AssumeInst>(II), II->bundle_op_info_begin()[Idx]);
if (!RK || RK.AttrKind != Attribute::Alignment ||
!isPowerOf2_64(RK.ArgValue))
continue;

// Don't try to remove align assumptions for pointers derived from
// arguments. We might lose information if the function gets inline and
// the align argument attribute disappears.
Value *UO = getUnderlyingObject(RK.WasOn);
if (!UO || isa<Argument>(UO))
continue;

KnownBits Known = computeKnownBits(RK.WasOn, 0, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on why the nullptr context here? Though I think we'd be better off adding an AllowEphemeralValues option to SimplifyQuery and use it as appropriate. We can safely set AllowEphemeralValues=true when inferring alignment and can drop the current special case for align operand bundles.

unsigned TZ = std::min(Known.countMinTrailingZeros(), 63u);
if ((1ULL << TZ) < RK.ArgValue)
continue;
auto *New = CallBase::removeOperandBundle(II, OBU.getTagID());
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary variable.

return New;
}
}

// Convert nonnull assume like:
Expand Down
56 changes: 53 additions & 3 deletions llvm/test/Transforms/InstCombine/assume-align.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals smart
; RUN: opt -S -passes=instcombine,simplifycfg < %s 2>&1 | FileCheck %s
; RUN: opt -S -passes='instcombine<no-verify-fixpoint>,simplifycfg' < %s 2>&1 | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the instcombine-no-verify-fixpoint function attribute instead, and add a comment why it's necessary.


declare void @llvm.assume(i1 noundef)

Expand Down Expand Up @@ -135,6 +135,17 @@ define ptr @fold_assume_align_pow2_of_loaded_pointer_into_align_metadata(ptr %p)
ret ptr %p2
}

define ptr @fold_assume_align_i32_pow2_of_loaded_pointer_into_align_metadata(ptr %p) {
; CHECK-LABEL: @fold_assume_align_i32_pow2_of_loaded_pointer_into_align_metadata(
; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P:%.*]], align 8
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[P2]], i32 8) ]
; CHECK-NEXT: ret ptr [[P2]]
;
%p2 = load ptr, ptr %p
call void @llvm.assume(i1 true) [ "align"(ptr %p2, i32 8) ]
ret ptr %p2
}

define ptr @dont_fold_assume_align_pow2_of_loaded_pointer_into_align_metadata_due_to_call(ptr %p) {
; CHECK-LABEL: @dont_fold_assume_align_pow2_of_loaded_pointer_into_align_metadata_due_to_call(
; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P:%.*]], align 8
Expand Down Expand Up @@ -175,7 +186,6 @@ define ptr @dont_fold_assume_align_zero_of_loaded_pointer_into_align_metadata(pt
define ptr @redundant_assume_align_1(ptr %p) {
; CHECK-LABEL: @redundant_assume_align_1(
; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P:%.*]], align 8
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[P2]], i32 1) ]
; CHECK-NEXT: call void @foo(ptr [[P2]])
; CHECK-NEXT: ret ptr [[P2]]
;
Expand All @@ -189,7 +199,6 @@ define ptr @redundant_assume_align_1(ptr %p) {
define ptr @redundant_assume_align_8_via_align_metadata(ptr %p) {
; CHECK-LABEL: @redundant_assume_align_8_via_align_metadata(
; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P:%.*]], align 8, !align [[META0:![0-9]+]]
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[P2]], i32 8) ]
; CHECK-NEXT: call void @foo(ptr [[P2]])
; CHECK-NEXT: ret ptr [[P2]]
;
Expand Down Expand Up @@ -249,7 +258,48 @@ define ptr @redundant_assume_align_8_via_asume(ptr %p) {
ret ptr %p
}

define void @redundant_arg_passed_to_intrinsic(ptr %dst, ptr %src) {
; CHECK-LABEL: @redundant_arg_passed_to_intrinsic(
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[DST:%.*]], i32 8) ]
; CHECK-NEXT: call void @bar()
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[SRC:%.*]], i32 8) ]
; CHECK-NEXT: call void @llvm.memmove.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(16) [[DST]], ptr noundef nonnull align 8 dereferenceable(16) [[SRC]], i64 16, i1 false)
; CHECK-NEXT: ret void
;
call void @llvm.assume(i1 true) [ "align"(ptr %dst, i32 8) ]
call void @bar()
call void @llvm.assume(i1 true) [ "align"(ptr %src, i32 8) ]
call void @llvm.memmove.p0.p0.i64(ptr align 8 %dst, ptr %src, i64 16, i1 false)
ret void
}

define void @test_store(ptr %ptr) {
; CHECK-LABEL: @test_store(
; CHECK-NEXT: entry:
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[PTR:%.*]], i64 2) ]
; CHECK-NEXT: store i16 0, ptr [[PTR]], align 1
; CHECK-NEXT: ret void
;
entry:
call void @llvm.assume(i1 true) [ "align"(ptr %ptr, i64 2) ]
store i16 0, ptr %ptr, align 1
ret void
}

declare void @foo(ptr)
declare void @bar()

; !align must have a constant integer alignment.
define ptr @dont_fold_assume_align_not_constant_of_loaded_pointer_into_align_metadata(ptr %p, i64 %align) {
; CHECK-LABEL: @dont_fold_assume_align_not_constant_of_loaded_pointer_into_align_metadata(
; CHECK-NEXT: [[P2:%.*]] = load ptr, ptr [[P:%.*]], align 8
; CHECK-NEXT: ret ptr [[P2]]
;
%p2 = load ptr, ptr %p
call void @llvm.assume(i1 true) [ "align"(ptr %p2, i64 %align) ]
ret ptr %p2
}

;.
; CHECK: [[META0]] = !{i64 8}
;.
8 changes: 4 additions & 4 deletions llvm/test/Transforms/InstCombine/assume.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -passes=instcombine -S | FileCheck --check-prefixes=CHECK,DEFAULT %s
; RUN: opt < %s -passes=instcombine --enable-knowledge-retention -S | FileCheck --check-prefixes=CHECK,BUNDLES %s
; RUN: opt < %s -passes='instcombine<no-verify-fixpoint>' -S | FileCheck --check-prefixes=CHECK,DEFAULT %s
; RUN: opt < %s -passes='instcombine<no-verify-fixpoint>' --enable-knowledge-retention -S | FileCheck --check-prefixes=CHECK,BUNDLES %s

; RUN: opt < %s -passes=instcombine -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK,DEFAULT %s
; RUN: opt < %s -passes=instcombine --enable-knowledge-retention -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK,BUNDLES %s
; RUN: opt < %s -passes='instcombine<no-verify-fixpoint>' -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK,DEFAULT %s
; RUN: opt < %s -passes='instcombine<no-verify-fixpoint>' --enable-knowledge-retention -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK,BUNDLES %s

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
Expand Down
Loading