Addressed review comments.

Addressed review comment by hard coding the prefix for MLS_DForm2_r0.

Address review comments.

Added full context.

Added full context.

Added full context.

Other than the test case which still needs to be updated I think that this patch looks good to me.

It took me a while to determine that this was in fact NFC.
Removing the whole:

if (HasAnyUndefs || ISD::isBuildVectorAllOnes(BVN)) {
...
}

threw me off.
I'm convinced now so I'll approve it.
The minor nit that I have can be done on commit.

Please add a test case for this.
Maybe an test were we have an SPE load of f64 with a 8 bit offset and one where the offset is larger or perhaps not aligned. Anyway, that's just a suggestion. You probably have a better idea of what you would like to test for this.

I think that for f64 on SPE the GprIndex you are computing it going to be ignored. (See my comment...)

I only saw a minor nit with this patch overall.

LGTM

LGTM

I think this patch is fine now.
I only have one minor comment.

Mostly minor comments and a couple of questions.

Please make sure that your test does not put restrictions on the allocation of output registers. See my comments.
Otherwise I think this looks fine.

LGTM.
Sorry for taking so long to get to this.

Thank you for fixing this.
LGTM.

I took this patch and applied it to master today and I noticed that one of the LIT tests fails.
The test is:

test-suite/MultiSource/Benchmarks/tramp3d-v4/

It segfaults with this patch and runs fine with out it.
I took a quick look at it and it seems that in this basic block:

; predecessors: %bb.11, %bb.12, %bb.13
  successors: %bb.21(0x80000000); %bb.21(100.00%)
  liveins: $r6, $r7, $r9, $x4, $x5, $x10, $x12, $x30, $r8
  renamable $x3 = ADDI8 $x1, 32
  STW killed renamable $r8, 32, $x1 :: (store 4 into %ir.arrayidx.i.i.i.i.i.i.i.i424, align 8), (store 4 into %ir.129, align 8)
  STW killed renamable $r6, 36, $x1 :: (store 4 into %ir.132)
  STW killed renamable $r7, 44, $x1 :: (store 4 into %ir.134)
  STW killed renamable $r9, 52, $x1 :: (store 4 into %ir.136)
  renamable $x3 = ADD8 killed renamable $x3, killed renamable $x5
  renamable $x5 = nsw ADD8 killed renamable $x10, killed renamable $x12
  renamable $r3 = LWZ 4, killed renamable $x3 :: (load 4 from %ir.137)
  renamable $x5 = nsw SUBF8 renamable $x4, killed renamable $x5
  STW8 killed renamable $x4, 24, renamable $x30 :: (store 4 into %ir.141)
  renamable $r3 = ADD4 killed renamable $r3, killed renamable $r5, implicit $x5
  B %bb.21

We replace

renamable $x3 = ADDI8 $x1, 32
renamable $x3 = ADD8 killed renamable $x3, killed renamable $x5
renamable $r3 = LWZ 4, killed renamable $x3 :: (load 4 from %ir.137)

with:

renamable $x3 = ADDI8 $x1, 36
renamable $r3 = LWZX killed $x5, killed $x3 :: (load 4 from %ir.137)

However, it looks like x5 is redefined before the load so using it in the LWZX is not safe because it is not the same x5 as before.

Thank you Galina!
Committed at r371652.

Thank you Galina!
Committed at r372342.

@gkistanova
Thank you for reviewing this Galina.
I don't have commit access to zorg. Can you commit this for me?

I do have a couple additional comments.

Sorry it took me so long to get back to this.
LGTM.

@gkistanova
Thank you for the review Galina.
I unfortunately don't have commit access to Zorg. Could you commit this for me?

Overall this looks good. I have a couple of very minor comments that can be fixed on commit.
LGTM.

The fix looks good to me.
However, I feel that the test is over-specified. What you are looking for is a rotate or a shift with an immediate of zero and to make sure that we can safely print those kinds of instructions to assembly. I feel that using the update_llc_test_checks.py script only produces a test that will fail when other changes are made later on and forcing the developer of that changeset to regenerate your test.

I like what this patch is doing. Overall, I only have some refactoring comments.

Added nit.

I only have a number of very minor comments. Overall I think this looks good.
I think you can fix the comments when you commit.

@gkistanova
Hi Galina,
We are having intermittent svn failures on our lnt build bot as a result of this http vs. https issue. (The bot: http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt)
Would it be possible to take a look at this patch because we think that this will fix our bot.
Thank you!

Committed by this patch:

commit 0a4f7b2312b55e8f3144cbb9e658bc37cc4fc779
Author: Galina Kistanova <gkistanova@gmail.com>
Date:   Mon Feb 25 20:19:36 2019 +0000

LGTM.

You have convinced me that this will only affect AIX. I only have minor comments remaining.

I feel like this is supposed to be an AIX only patch.
However, there are some cases where you are making changes to PowerPC platforms that are not AIX. I don't know if those changes are safe.
I'm going to add two people to the review who may be interested in this and who work on 32 Bit PowerPC.

I'm not super familiar with the MC layer but overall this looks good to me.
I have a couple of minor comments.

I only have one comment here and I think it can be fixed when the patch is committed.
LGTM.

LGTM.
The one very minor comment I have I think can be addressed on commit.

I have a couple of nits. The main one being the test case for fma-mutate.ll.
I'm not sure if there is a way to use one const NR for just that one test.

Looks good to me.
Then only thing I noticed was that in the description in the LanguageExtensions.rst file you use plural to refer to the chache builtins. Of course, you only added one builtin but I know that in the future you plan to add more builtins of the same type so I wouldn't worry about that wording now.

The patch does what we want it to do in most cases (including the example that motivated it) but some boundary conditions need to be considered before it is safe in all cases.

@nemanjai
I will commit this for Victor.

Have a couple of minor nits about the test case but that is all.
LGTM.

LGTM

Minor nit that can be fixed when this patch is committed.
LGTM.

The reason why we don't clean up that extra store on no-PIC is because we run a couple of extra passes for PIC for PowerPC.
It is this code:

// FIXME: We probably don't need to run these for -fPIE.
if (getPPCTargetMachine().isPositionIndependent()) {
  // FIXME: LiveVariables should not be necessary here!
  // PPCTLSDynamicCallPass uses LiveIntervals which previously dependent on
  // LiveVariables. This (unnecessary) dependency has been removed now,
  // however a stage-2 clang build fails without LiveVariables computed here.
  addPass(&LiveVariablesID, false);
  addPass(createPPCTLSDynamicCallPass());
}

Those extra passes clean up the extra store.
If I remember correctly this is only really an issue at -O0. At higher opts other passes clean up stores like that.

@hfinkel @nemanjai
Hi,
Sorry for the late reply. It took me a little while to figure out that I had specified the wrong options for my test when trying to reproduce this....
Anyway, I've verified that what Nemanja said is indeed correct:
no-PIC

bl      100006b0 <00000017.plt_call.omp_control_tool@@VERSION>                                       
ld      r2,24(r1)                                                                                    
nop                                                                                                  
stw     r3,108(r31)
nop

PIC

bl      100006b0 <00000017.plt_call.omp_control_tool@@VERSION>
ld      r2,24(r1)
nop
nop

The store in the no-PIC version is just saving the return value from r3.