This is an archive of the discontinued LLVM Phabricator instance.

Change the testcase tail-merge-after-mbp.ll to tail-merge-after-mbp.mir
ClosedPublic

Authored by haicheng on Apr 13 2017, 11:23 AM.

Details

Summary

I found tail-merge-after-mbp.ll as a .ll test was fragile to the changes in preivous pipeline, so I like to change it to a .mir test and run it with only block-placement pass.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi created this revision.Apr 13 2017, 11:23 AM
gberry edited edge metadata.

I believe a lot of the IR in this test case is superfluous (as well as some of the MIR property flags like hasStackMap, etc.). Could you try cutting it down to only the necessary IR and MIR to make it more robust against future changes?
I've added Matthias as a reviewer since he is more familiar with the current state of the MIR parser.

Sorry if I wasn't clear. I'm not suggesting you change what is being tested, just that you remove code from the test that doesn't impact it. For example, I believe you can get rid of all of the IR code if you also remove the MemoryOperands info from the MIR (the :: (load 8 from xxx) stuff).

haicheng edited edge metadata.Apr 13 2017, 1:38 PM

Block 3 4 7 9 in IR are not necessary. The generated MIR can be manually reduced as Geoff said. If you can wait till early next week, I can simplify this test case for you.

wmi updated this revision to Diff 95205.Apr 13 2017, 1:54 PM

Remove some code from the test that doesn't impact it.

MatzeB edited edge metadata.Apr 13 2017, 2:04 PM

The question of reducing .mir files keeps popping up, I will add some tips and tricks to the llvm documentation.

The question of reducing .mir files keeps popping up, I will add some tips and tricks to the llvm documentation.

I wrote some paragraphs about simplifying .mir tests: https://reviews.llvm.org/D32058

(We really should have tools for some things like dropping IR references, but until we do have tools there is at least a description of the process now).

haicheng commandeered this revision.Apr 17 2017, 2:04 PM
haicheng edited reviewers, added: wmi; removed: haicheng.
haicheng updated this revision to Diff 95483.Apr 17 2017, 2:05 PM

Simplified the mir test case a little bit.

MatzeB accepted this revision.Apr 17 2017, 2:09 PM

This looks like you can remove the whole IR block (--- | to ... included).

In any case LGTM, thanks.

This revision is now accepted and ready to land.Apr 17 2017, 2:09 PM
haicheng updated this revision to Diff 95486.Apr 17 2017, 2:15 PM

Remove the whole IR module.

wmi edited edge metadata.Apr 17 2017, 2:45 PM

Haicheng, the testcase LGTM. Thanks for working on it!

This revision was automatically updated to reflect the committed changes.