Ping? @vsk Do you think it worth updating Bugpoint.rst? If so, any comments? Thanks.

Hmm... OK, too many details that I did not even think of. I would leave this to be reviewed by other experts.

LGTM.

Can you please use MIR test instead. Thanks.

In D67167#1667618, @jmolloy wrote:

Thanks for the great feedback Jinsong! I've addressed your comments.

The patch is only updating existing tests in three places -- which to me looks more like a happy coincidence (that LLVM already includes tests that happen to trigger the new behavior) than intentional testing. Thanks!

Please add more tests as required by @gribozavr . Thanks.

LGTM. May need @kbarton to have another look, as he should be in the "Must Review" for this.

LGTM.

> I see the need to make error validation with bugpoint more beginner-friendly, but am worried about situations in which the mode proposed here could confuse beginners (what if the first error isn't the interesting one? or reductions stop too early?). I don't have a better alternative to suggest. Perhaps we could repost @mehdi_amini's writeup in the "Search for a string in the output" section within http://blog.llvm.org/2015/11/reduce-your-testcases-with-bugpoint-and.html, or link to it from the bugpoint docs.

Abandon the --same-error change, update docs instead.

I think it would be great to make the powerful bugpoint tool easier to use, especially for beginners, for common simple scenarios.

Thanks @vsk for the valuable comments.

Please include *Full Context* in newer diff . Thanks.

General idea is great, some comments when I looked at PowerPC part. Thanks.

Ping..
and add the most recent code modifier & patch reviewers . Thanks!

Rebase to ToT.

LGTM.

https://reviews.llvm.org/rL370228 just landed, so I think you should include fptrunc and fpext intrinsics as well.

LGTM. Please hold for a few days, in case someone has more comments. Thanks.

Add range check and tests.

Add range check and test.

Ping ...

Ping..

Ping..

My understanding is that the cycle saving come from avoiding unnecessary SP->DP, then DP->SP conversion.
But not the difference merge sequence.

I'll take that you accepted the patch, and commit it, so that @adalava can quickly file a merge request to release_90.

Added encoding tests.

In D66963#1651477, @hfinkel wrote:

mfvrwz RA,VRS is mfvsrwz RA,VRS+32

The '+32' bit is covered by the added CodeGen/PowerPC/inlineasm-extendedmne.ll test, but I'd really like to see direct encoding tests for all of the instructions. Can you please make sure that they all appear in MC/PowerPC/vsx.s and MC/Disassembler/PowerPC/vsx.txt?

The change looks fine to me, as LLVM currently doesn't generate relocation pairs.

Yes, the list in VecFuncs.def could be reused in PPCISelLowering. But, PPCISelLowering.cpp is already big as it is. Adding more functions to handle this lowering will only add to its size and a list of features it already supports. Therefore, my preference is to separate out the MASSV lowering functionality into a separate transformation.

1. *_massv functions serve as place holder for math functions during loop vectorization. They are not actual entries in the MASS library. I think lowering them to actual entries in the MASS library deserves a separate pass rather than polluting the TargetLowering.cpp, which is already big as it is.

A separate pass for MASSV lowering is worthwhile because it allows better separation of concerns.
In the future, if need be, other PPC sub-target specific lowering decisions for MASSV may be encapsulated in this pass too.

Move out of review queue, need author's action.

Move out of review queue, need author's action.

Are you still pursuing this Patch?

Fix typos in testcase.

In D63972#1646303, @skatkov wrote:

Please provide a quick fix or revert the patch causing the crash due to transforming to incorrect IR.