This is an archive of the discontinued LLVM Phabricator instance.

New Test-Case for Region Analysis
ClosedPublic

Authored by nandini12396 on Mar 4 2017, 2:54 AM.

Details

Summary

While working on improvements to the region info analysis, this test case caused an incorrect region bb2 => bb3 to be detected.
It is incorrect because bb2 has an outgoing edge to bb4.
This is interesting because bb2 dom bb3 and bb3 pdom bb2, which should have been enough to prevent incoming forward edges into the region and outgoing forward edges from the region.

Diff Detail

Event Timeline

nandini12396 created this revision.Mar 4 2017, 2:54 AM
nandini12396 edited the summary of this revision. (Show Details)
grosser edited edge metadata.Mar 5 2017, 2:15 AM

Hi Nandini,

thanks for the test case. Could you please run '-instnamer' over the test case, to get easier readable and modifiable instruction names. Please add also CHECK lines that verify that the output of the region analysis is correct.

Best,
Tobias

grosser requested changes to this revision.Mar 5 2017, 2:15 AM
This revision now requires changes to proceed.Mar 5 2017, 2:15 AM
nandini12396 edited edge metadata.

Sorry for the delay.

The output of opt -instnamer was the same as before (maybe because the bb names are not tmp?).

Hi Nandini,

it seems your patch indeed renamed the basic block names from "0" (which means no name) to "bb0". That was precisely the effect I expected.

Now, you now unfortunately only submitted the diff between the previous and the new state of your change, instead of the diff between the current LLVM trunk and your patch. This is a common misunderstanding with phabricator. What you want to do is to rebase your patch and merge the different patches into a single one, which you then submit as new update of this patch. Could you please do this final update, such that I can then commit your patch.

Best,
Tobias

This revision was automatically updated to reflect the committed changes.

Thank you Nandini!

nandini12396 edited the summary of this revision. (Show Details)Mar 5 2017, 6:26 AM

Thank you Nandini!

Respected Sir,

Thank you.

Regards
Nandini