This is an archive of the discontinued LLVM Phabricator instance.

Differential D15970

While adding instructions to be redone, make sure they are from the same basic block
ClosedPublic

Authored by aditya_nandakumar on Jan 7 2016, 2:41 PM.

Download Raw Diff

Details

Reviewers

mcrosier

Summary

While adding instructions (possible roots) to be redone, make sure they are from the same basic block.

Attached test case where it will crash without this patch.

Diff Detail

Repository: rL LLVM

Event Timeline

aditya_nandakumar updated this revision to Diff 44259.Jan 7 2016, 2:41 PM

aditya_nandakumar retitled this revision from to While adding instructions to be redone, make sure they are from the same basic block.

aditya_nandakumar updated this object.

aditya_nandakumar added a reviewer: mcrosier.

aditya_nandakumar set the repository for this revision to rL LLVM.

LGTM.

This revision is now accepted and ready to land.Jan 7 2016, 3:21 PM

One more thing: Make sure to add llvm-commits to the subscribers list going forward.

mcrosier edited edge metadata.Jan 7 2016, 3:25 PM

mcrosier added a subscriber: llvm-commits.

Ok. Will do. Thanks. Submitting.

aditya_nandakumar closed this revision.Jan 7 2016, 3:50 PM

Revision Contents

Path

Size

lib/

Transforms/

Scalar/

Reassociate.cpp

3 lines

test/

Transforms/

Reassociate/

add_across_block_crash.ll

20 lines

Diff 44259

lib/Transforms/Scalar/Reassociate.cpp

Show First 20 Lines • Show All 2,149 Lines • ▼ Show 20 Lines	void Reassociate::OptimizeInst(Instruction *I) {

// If this is an interior node of a reassociable tree, ignore it until we		// If this is an interior node of a reassociable tree, ignore it until we
// get to the root of the tree, to avoid N^2 analysis.		// get to the root of the tree, to avoid N^2 analysis.
unsigned Opcode = BO->getOpcode();		unsigned Opcode = BO->getOpcode();
if (BO->hasOneUse() && BO->user_back()->getOpcode() == Opcode) {		if (BO->hasOneUse() && BO->user_back()->getOpcode() == Opcode) {
// During the initial run we will get to the root of the tree.		// During the initial run we will get to the root of the tree.
// But if we get here while we are redoing instructions, there is no		// But if we get here while we are redoing instructions, there is no
// guarantee that the root will be visited. So Redo later		// guarantee that the root will be visited. So Redo later
if (BO->user_back() != BO)		if (BO->user_back() != BO &&
		BO->getParent() == BO->user_back()->getParent())
RedoInsts.insert(BO->user_back());		RedoInsts.insert(BO->user_back());
return;		return;
}		}

// If this is an add tree that is used by a sub instruction, ignore it		// If this is an add tree that is used by a sub instruction, ignore it
// until we process the subtract.		// until we process the subtract.
if (BO->hasOneUse() && BO->getOpcode() == Instruction::Add &&		if (BO->hasOneUse() && BO->getOpcode() == Instruction::Add &&
cast<Instruction>(BO->user_back())->getOpcode() == Instruction::Sub)		cast<Instruction>(BO->user_back())->getOpcode() == Instruction::Sub)
▲ Show 20 Lines • Show All 139 Lines • Show Last 20 Lines

test/Transforms/Reassociate/add_across_block_crash.ll

This file was added.

				; RUN: opt < %s -reassociate -S \| FileCheck %s
				; CHECK-LABEL: main
				; This test is to make sure while processing a block, uses of instructions
				; from a different basic block don't get added to be re-optimized
				define void @main() {
				entry:
				%0 = fadd fast float undef, undef
				br i1 undef, label %bb1, label %bb2

				bb1:
				%1 = fmul fast float undef, -2.000000e+00
				%2 = fmul fast float %1, 2.000000e+00
				%3 = fadd fast float %2, 2.000000e+00
				%4 = fadd fast float %3, %0
				%mul351 = fmul fast float %4, 5.000000e-01
				ret void

				bb2:
				ret void
				}