Merge r339260 into the 7.0 branch

[nemanjai]

Bugzilla Link	39295
Resolution	FIXED
Resolved on	Nov 29, 2018 20:51
Version	trunk
OS	All
Blocks	#38454
CC	@hfinkel,@tstellar
Fixed by commit(s)	r339260 r347957

Extended Description

An issue was reported to us internally by a customer. We had an upcoming fix for it so didn't open a bug since the fix had already been planned to go upstream. It turned out that we committed this after the release split.
This was an oversight on my end. We need this merged into 7.0.1.

[nemanjai]

assigned to @hfinkel

[tstellar]

Does this fix a bug or is it purely an optimization?

[nemanjai]

It does fix a bug in the client's proprietary code. The client has confirmed the fix when it was committed but we just forgot to ask for this to be merged in the release branch in time for the 7.0.0 release.

[tstellar]

Hi Hal,

Is this OK to merge?

https://reviews.llvm.org/rL339260

[hfinkel]

Hi Hal,

Is this OK to merge?

https://reviews.llvm.org/rL339260

Nemanja, what was the bug?

This commit looks like it is intended to be a code-quality enhancement, and is, but it's non-trivial. It adds several new patterns, some Endian-dependent, and changes other logic, and pulling this into a release branch makes me somewhat nervous.

[nemanjai]

Ah, yes I remember this one.
It was with the code that prevents us from emitting a splat when it is fed by a 32-bit load (with the assumption that LXVWSX will be emitted for that in every case). However, there is a case where we won't emit LXVWSX and it is when the 32-bit load is a pre-inc load (since we don't have an update form of this instruction).

Normally, we would have pulled that out of the patch and committed separately as a bug fix, but the timing of the report was kind of unfortunate.

If I recall correctly, they had contacted me, I did a bit of investigating and realized that this patch will likely fix it. By the time they had a chance to verify that this fixes it, the patch had already been upstreamed (and they also reported that ToT at the time did not exhibit the problem).

I realize that this is kind of a weird patch to push into the release branch and I apologize for this mixup.

[hfinkel]

Ah, yes I remember this one.
It was with the code that prevents us from emitting a splat when it is fed
by a 32-bit load (with the assumption that LXVWSX will be emitted for that
in every case). However, there is a case where we won't emit LXVWSX and it
is when the 32-bit load is a pre-inc load (since we don't have an update
form of this instruction).

Normally, we would have pulled that out of the patch and committed
separately as a bug fix, but the timing of the report was kind of
unfortunate.

If I recall correctly, they had contacted me, I did a bit of investigating
and realized that this patch will likely fix it. By the time they had a
chance to verify that this fixes it, the patch had already been upstreamed
(and they also reported that ToT at the time did not exhibit the problem).

I realize that this is kind of a weird patch to push into the release branch
and I apologize for this mixup.

We can pull this in; I definitely understand how timing of these things can be unfortunate. Under the circumstances, pulling this in is probably the best option. It's been okay in trunk for some time now.

[tstellar]

Merged: r347957