This is an archive of the discontinued LLVM Phabricator instance.

Differential D101226

[SimplifyCFG] Preserve metadata when unconditionalizing branches (same target).
ClosedPublic

Authored by Meinersbur on Apr 23 2021, 11:57 PM.

Details

Reviewers
jeroen.dobbelaere
lebedev.ri
Commits
rGb99466eb4525: [SimplifyCFG] Preserve metadata when unconditionalizing branches (same target).
Summary

When replacing a conditional branch by an unconditional one because the targets are identical, transfer the metadata to the new branch instruction.

Diff Detail

Unit TestsFailed

TimeTest
150 msx64 windows > lld.MachO::reproduce.s

Event Timeline

Meinersbur created this revision.Apr 23 2021, 11:57 PM
Herald added a subscriber: hiraditya. · View Herald TranscriptApr 23 2021, 11:57 PM
Meinersbur requested review of this revision.Apr 23 2021, 11:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2021, 11:57 PM
Meinersbur mentioned this in D101141: [SimplifyCFG] Preserve metadata when unconditionalizing branches (constant condition)..Apr 23 2021, 11:58 PM
Harbormaster completed remote builds in B100738: Diff 340245.Apr 24 2021, 12:40 AM
Meinersbur updated this revision to Diff 340417.Apr 25 2021, 7:29 PM
  • Transfer only selected metadata kinds.
Meinersbur updated this revision to Diff 340419.Apr 25 2021, 7:36 PM
  • clang-format
Harbormaster completed remote builds in B100851: Diff 340417.Apr 25 2021, 8:06 PM
Harbormaster completed remote builds in B100853: Diff 340419.Apr 25 2021, 8:27 PM
lebedev.ri accepted this revision.Apr 26 2021, 12:21 AM

This LGTM, thank you.

llvm/lib/Transforms/Utils/Local.cpp
155

I'm guessing you are intentionally not copying mustprogress?

This revision is now accepted and ready to land.Apr 26 2021, 12:21 AM
Meinersbur added inline comments.Apr 26 2021, 3:19 PM
llvm/lib/Transforms/Utils/Local.cpp
155

As @jeroen.dobbelaere mentioned in D101141, mustprogress is a property inside the MD_loop metadata.

Closed by commit rGb99466eb4525: [SimplifyCFG] Preserve metadata when unconditionalizing branches (same target). (authored by Meinersbur). · Explain WhyApr 26 2021, 3:23 PM
This revision was automatically updated to reflect the committed changes.
Meinersbur added a commit: rGb99466eb4525: [SimplifyCFG] Preserve metadata when unconditionalizing branches (same target)..

Revision Contents

PathSize
llvm/
lib/
Transforms/
Utils/
6 lines
test/
Transforms/
SimplifyCFG/
19 lines

Diff 340417

llvm/lib/Transforms/Utils/Local.cpp

Show First 20 Lines • Show All 142 Lines • ▼ Show 20 Lines if (Dest2 == Dest1) { // Conditional branch to same location?
// br bool %cond, label %Dest, label %Dest // br bool %cond, label %Dest, label %Dest
// and changes it into: br label %Dest // and changes it into: br label %Dest
// Let the basic block know that we are letting go of one copy of it. // Let the basic block know that we are letting go of one copy of it.
assert(BI->getParent() && "Terminator not inserted in block!"); assert(BI->getParent() && "Terminator not inserted in block!");
Dest1->removePredecessor(BI->getParent()); Dest1->removePredecessor(BI->getParent());
// Replace the conditional branch with an unconditional one. // Replace the conditional branch with an unconditional one.
Builder.CreateBr(Dest1); BranchInst *NewBI = Builder.CreateBr(Dest1);
// Transfer the metadata to the new branch instruction.
NewBI->copyMetadata(*BI, {LLVMContext::MD_loop, LLVMContext::MD_dbg, LLVMContext::MD_annotation});
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-      NewBI->copyMetadata(*BI, {LLVMContext::MD_loop, LLVMContext::MD_dbg,        LLVMContext::MD_annotation});
+      NewBI->copyMetadata(*BI, {LLVMContext::MD_loop, LLVMContext::MD_dbg,
+                                LLVMContext::MD_annotation});
Lint: Pre-merge checks: clang-format: please reformat the code ``` - NewBI->copyMetadata(*BI, {LLVMContext…
lebedev.riUnsubmitted
Not Done
ReplyInline Actions

I'm guessing you are intentionally not copying mustprogress?

lebedev.ri: I'm guessing you are intentionally not copying `mustprogress`?
MeinersburAuthorUnsubmitted
Done
ReplyInline Actions

As @jeroen.dobbelaere mentioned in D101141, mustprogress is a property inside the MD_loop metadata.

Meinersbur: As @jeroen.dobbelaere mentioned in D101141, `mustprogress` is a property inside the `MD_loop`…
Value *Cond = BI->getCondition(); Value *Cond = BI->getCondition();
BI->eraseFromParent(); BI->eraseFromParent();
if (DeleteDeadConditions) if (DeleteDeadConditions)
RecursivelyDeleteTriviallyDeadInstructions(Cond, TLI); RecursivelyDeleteTriviallyDeadInstructions(Cond, TLI);
return true; return true;
} }
if (auto *Cond = dyn_cast<ConstantInt>(BI->getCondition())) { if (auto *Cond = dyn_cast<ConstantInt>(BI->getCondition())) {
▲ Show 20 Lines • Show All 3,216 Lines • Show Last 20 Lines

llvm/test/Transforms/SimplifyCFG/commondest-loopid.ll

  • This file was added.
; RUN: opt < %s -simplifycfg -S | FileCheck %s
;
; Ensure that the loop metadata is preserved when converting the
; conditional branch to an unconditional.
define void @commondest_loopid(i1 %T) {
; CHECK-LABEL: @commondest_loopid(
; CHECK: !llvm.loop !0
; CHECK: !0 = distinct !{!0, !1}
; CHECK: !1 = !{!"loopprop"}
entry:
br label %loop
loop:
br i1 %T, label %loop, label %loop, !llvm.loop !0
}
!0 = distinct !{!0, !1}
!1 = !{!"loopprop"}