Skip to content

Commit

Permalink
[SimplifyCFG][LICM] Preserve nonnull, range and align metadata when s…
Browse files Browse the repository at this point in the history
…peculating

After D141386, violation of nonnull, range and align metadata
results in poison rather than immediate undefined behavior,
which means that these are now safe to retain when speculating.
We only need to remove UB-implying metadata like noundef.

This is done by adding a dropUBImplyingAttrsAndMetadata() helper,
which lists the metadata which is known safe to retain on speculation.

Differential Revision: https://reviews.llvm.org/D146629
  • Loading branch information
nikic committed Apr 4, 2023
1 parent f7deb69 commit 78b1fbc
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 21 deletions.
7 changes: 6 additions & 1 deletion llvm/include/llvm/IR/Instruction.h
Expand Up @@ -401,11 +401,16 @@ class Instruction : public User,
}

/// This function drops non-debug unknown metadata (through
/// dropUnknownNonDebugMetadata). For calls, it also drops parameter and
/// dropUnknownNonDebugMetadata). For calls, it also drops parameter and
/// return attributes that can cause undefined behaviour. Both of these should
/// be done by passes which move instructions in IR.
void dropUBImplyingAttrsAndUnknownMetadata(ArrayRef<unsigned> KnownIDs = {});

/// Drop any attributes or metadata that can cause immediate undefined
/// behavior. Retain other attributes/metadata on a best-effort basis.
/// This should be used when speculating instructions.
void dropUBImplyingAttrsAndMetadata();

/// Determine whether the exact flag is set.
bool isExact() const LLVM_READONLY;

Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/IR/Instruction.cpp
Expand Up @@ -243,6 +243,16 @@ void Instruction::dropUBImplyingAttrsAndUnknownMetadata(
CB->removeRetAttrs(UBImplyingAttributes);
}

void Instruction::dropUBImplyingAttrsAndMetadata() {
// !annotation metadata does not impact semantics.
// !range, !nonnull and !align produce poison, so they are safe to speculate.
// !noundef and various AA metadata must be dropped, as it generally produces
// immediate undefined behavior.
unsigned KnownIDs[] = {LLVMContext::MD_annotation, LLVMContext::MD_range,
LLVMContext::MD_nonnull, LLVMContext::MD_align};
dropUBImplyingAttrsAndUnknownMetadata(KnownIDs);
}

bool Instruction::isExact() const {
return cast<PossiblyExactOperator>(this)->isExact();
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Scalar/LICM.cpp
Expand Up @@ -1757,7 +1757,7 @@ static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
// time in isGuaranteedToExecute if we don't actually have anything to
// drop. It is a compile time optimization, not required for correctness.
!SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop))
I.dropUBImplyingAttrsAndUnknownMetadata();
I.dropUBImplyingAttrsAndMetadata();

if (isa<PHINode>(I))
// Move the new node to the end of the phi list in the destination block.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Utils/Local.cpp
Expand Up @@ -2989,7 +2989,7 @@ void llvm::hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt,

for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE;) {
Instruction *I = &*II;
I->dropUBImplyingAttrsAndUnknownMetadata();
I->dropUBImplyingAttrsAndMetadata();
if (I->isUsedByMetadata())
dropDebugUsers(*I);
if (I->isDebugOrPseudoInst()) {
Expand Down
14 changes: 5 additions & 9 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Expand Up @@ -1117,16 +1117,12 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
VMap[&BonusInst] = NewBonusInst;

// If we moved a load, we cannot any longer claim any knowledge about
// its potential value. The previous information might have been valid
// If we speculated an instruction, we need to drop any metadata that may
// result in undefined behavior, as the metadata might have been valid
// only given the branch precondition.
// For an analogous reason, we must also drop all the metadata whose
// semantics we don't understand. We *can* preserve !annotation, because
// it is tied to the instruction itself, not the value or position.
// Similarly strip attributes on call parameters that may cause UB in
// location the call is moved to.
NewBonusInst->dropUBImplyingAttrsAndUnknownMetadata(
LLVMContext::MD_annotation);
NewBonusInst->dropUBImplyingAttrsAndMetadata();

NewBonusInst->insertInto(PredBlock, PTI->getIterator());
NewBonusInst->takeName(&BonusInst);
Expand Down Expand Up @@ -3014,7 +3010,7 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
}

// Metadata can be dependent on the condition we are hoisting above.
// Conservatively strip all metadata on the instruction. Drop the debug loc
// Strip all UB-implying metadata on the instruction. Drop the debug loc
// to avoid making it appear as if the condition is a constant, which would
// be misleading while debugging.
// Similarly strip attributes that maybe dependent on condition we are
Expand All @@ -3025,7 +3021,7 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
if (!isa<DbgAssignIntrinsic>(&I))
I.setDebugLoc(DebugLoc());
}
I.dropUBImplyingAttrsAndUnknownMetadata();
I.dropUBImplyingAttrsAndMetadata();

// Drop ephemeral values.
if (EphTracker.contains(&I)) {
Expand Down
9 changes: 6 additions & 3 deletions llvm/test/Transforms/LICM/hoist-metadata.ll
Expand Up @@ -3,6 +3,7 @@

declare void @foo(...) memory(none)

; We can preserve all metadata on instructions that are guaranteed to execute.
define void @test_unconditional(i1 %c, ptr dereferenceable(8) align 8 %p) {
; CHECK-LABEL: define void @test_unconditional
; CHECK-SAME: (i1 [[C:%.*]], ptr align 8 dereferenceable(8) [[P:%.*]]) {
Expand All @@ -29,12 +30,14 @@ exit:
ret void
}

; We cannot preserve UB-implying metadata on instructions that are speculated.
; However, we can preserve poison-implying metadata.
define void @test_conditional(i1 %c, i1 %c2, ptr dereferenceable(8) align 8 %p) {
; CHECK-LABEL: define void @test_conditional
; CHECK-SAME: (i1 [[C:%.*]], i1 [[C2:%.*]], ptr align 8 dereferenceable(8) [[P:%.*]]) {
; CHECK-NEXT: [[V1:%.*]] = load i32, ptr [[P]], align 4
; CHECK-NEXT: [[V2:%.*]] = load ptr, ptr [[P]], align 8
; CHECK-NEXT: [[V3:%.*]] = load ptr, ptr [[P]], align 8
; CHECK-NEXT: [[V1:%.*]] = load i32, ptr [[P]], align 4, !range [[RNG0]]
; CHECK-NEXT: [[V2:%.*]] = load ptr, ptr [[P]], align 8, !nonnull !1
; CHECK-NEXT: [[V3:%.*]] = load ptr, ptr [[P]], align 8, !align !2
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: br i1 [[C]], label [[IF:%.*]], label [[LATCH:%.*]]
Expand Down
18 changes: 12 additions & 6 deletions llvm/test/Transforms/SimplifyCFG/hoist-with-metadata.ll
Expand Up @@ -61,10 +61,11 @@ out:
ret void
}

; !range violation only returns poison, and is thus safe to speculate.
define i32 @speculate_range(i1 %c, ptr dereferenceable(8) align 8 %p) {
; CHECK-LABEL: @speculate_range(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[P:%.*]], align 4, !range [[RNG2:![0-9]+]]
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[C:%.*]], i32 [[V]], i32 0
; CHECK-NEXT: ret i32 [[SPEC_SELECT]]
;
Expand All @@ -80,10 +81,12 @@ join:
ret i32 %phi
}

; !nonnull is safe to speculate, but !noundef is not, as the latter causes
; immediate undefined behavior.
define ptr @speculate_nonnull(i1 %c, ptr dereferenceable(8) align 8 %p) {
; CHECK-LABEL: @speculate_nonnull(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[V:%.*]] = load ptr, ptr [[P:%.*]], align 8
; CHECK-NEXT: [[V:%.*]] = load ptr, ptr [[P:%.*]], align 8, !nonnull !1
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[C:%.*]], ptr [[V]], ptr null
; CHECK-NEXT: ret ptr [[SPEC_SELECT]]
;
Expand All @@ -99,11 +102,12 @@ join:
ret ptr %phi
}


; !align is safe to speculate, but !dereferenceable is not, as the latter causes
; immediate undefined behavior.
define ptr @speculate_align(i1 %c, ptr dereferenceable(8) align 8 %p) {
; CHECK-LABEL: @speculate_align(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[V:%.*]] = load ptr, ptr [[P:%.*]], align 8
; CHECK-NEXT: [[V:%.*]] = load ptr, ptr [[P:%.*]], align 8, !align !3
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[C:%.*]], ptr [[V]], ptr null
; CHECK-NEXT: ret ptr [[SPEC_SELECT]]
;
Expand All @@ -122,7 +126,7 @@ join:
define void @hoist_fpmath(i1 %c, double %x) {
; CHECK-LABEL: @hoist_fpmath(
; CHECK-NEXT: if:
; CHECK-NEXT: [[T:%.*]] = fadd double [[X:%.*]], 1.000000e+00, !fpmath !2
; CHECK-NEXT: [[T:%.*]] = fadd double [[X:%.*]], 1.000000e+00, !fpmath !4
; CHECK-NEXT: ret void
;
if:
Expand All @@ -143,5 +147,7 @@ out:
;.
; CHECK: [[RNG0]] = !{i8 0, i8 1, i8 3, i8 5}
; CHECK: [[META1:![0-9]+]] = !{}
; CHECK: [[META2:![0-9]+]] = !{float 2.500000e+00}
; CHECK: [[RNG2]] = !{i32 0, i32 10}
; CHECK: [[META3:![0-9]+]] = !{i64 4}
; CHECK: [[META4:![0-9]+]] = !{float 2.500000e+00}
;.

0 comments on commit 78b1fbc

Please sign in to comment.