The "recent patch" mentioned in the description is https://reviews.llvm.org/D53346.

Actually, the handling for v2i32 is already optimal due to the DAG combine that handles this. We'll leave it in the DAG combine since that handles extractions even if they didn't come from legalizing v2i32 types (i.e. it handles a superset of the legalization added here).

Removed the actual definitions of the static arrays from the header. Simply provided the macro for the defs in the header and the macro is used in only the two CU's that need it.
This way if we need to add any new ones, they just need to be added to the macro.

LGTM now that we don't bitcast types any longer.

In D54449#1296245, @dschuff wrote:

Conceptually, COMPILER_RT_ABI is the calling convention used by all compiler-rt functions. It is explicitly named and used because it may be different from the default calling convention on some targets. In practice I believe it only affects ARM, where the calling convention uses ARM's soft-float ABI (where IIRC floats are passed in integer registers) even on targets that use the hard-float ABI by default. The compiler knows the calling conventions and emits the right code when it emits the libcalls, and the ABI declaration on the implementations causes the right calling convention to be used there. The unusual thing about the tests is that they locally declare the compiler-rt functions explicitly. So this declaration has to match the implementation. It may not matter for PPC but my recommendation would be "just use it everywhere in compiler-rt" for simplicity.

This is how I meant to do it initially but was reluctant to introduce a new header dependency. However, I don't think it's an issue at all for PPCMCTargetDesc.h to have a dependency on MC/MCRegisterInfo.h.

Did the same for the disassembler.

@hubert.reinterpretcast Have your comments been addressed adequately in the latest version of the patch? Do you have an opinion on adding the test case I proposed?

Just for clarification (and please add the text to the commit message), this is actually required by the ABI:

Each element of the result vector is the result of logically right shifting the corresponding
element of ARG1 by the number of bits specified by the value of the corresponding
element of ARG2, modulo the number of bits in the element. The bits that are shifted out
are replaced by zeros.

The test case fix can be done on the commit. Approving this now.

Since this fixes the sanitizers on PPC for the newer kernels and the fix seems perfectly reasonable and there are no concerns brought up by anyone, let's go ahead with this fix.

Please revert this. The way this was committed is incorrect. You've created directories such as llvm/trunk/llvm/ which we do not want. I suppose this has something to do with the fact that the patch was created using the monorepo and committed in some different way.

LGTM. Thanks.

Since the logic in the switch statement is difficult to follow, I ended up having to test this patch with a script that tries all combinations of comparisons, results and operand order in order to convince myself that it doesn't emit a setb when it is not valid to do so. Everything checks out there, but I don't really have an easy way of testing whether any opportunities are missed.
I am really hoping that you can simplify the logic in that switch to make it easier to follow.

I think the changes that are needed are clear enough that this doesn't require another review cycle. Approving this with the assumption that the required change will be made on the commit.

LGTM.

Updated the regular expression to just eat any text that is not relevant between the function directive and its text. This seems to work for both ELFv1 and ELFv2 asm output.

Implemented all the patterns for codegen of extract->store.

Modified the changeset to not reduce the capabilities of the pass (select the correct opcode based on the register for post-ra).

Other than the minor nit, LGTM.

LGTM. I like this approach of committing the test case up front so that the patch can show the differences in code gen clearly.

TL; DR: +1 from me for proceeding with this patch. If you don't hear any strong objections before next week, I'd say please update the test cases and we'll go ahead with this.

Thanks for your comments @MaskRay. Let me know if you think that RE would work.

@steven.zhang Seems I was mistaken about your nick on Phabricator. The above comment is meant for you Steven.

@qshanz I suspect this patch is obsolete now with your recent work on improving the r+r -> r+i transformations. If it is, I'll abandon it. If this still does something useful, we should update it and re-post.

Update to account for already committed PPC-specific changes.

Can we get this committed? This is good code cleanup that also fixes crashes (albeit ones that nobody has come across in the field).
The subsequent patch is controversial and we can deal with it separately, but this portion is non-controversial and should go upstream.

In D44548#1258889, @spatel wrote:

How about enabling the hook for PPC before adding to the generic DAG combine...
Right now, you'll get something horrible for normal fabs (and fneg?) without this patch, right?

	xscvdpspn 0, 1
	xxsldwi 0, 0, 0, 3
	mfvsrwz 3, 0
	rlwinm 3, 3, 0, 1, 31
	mtvsrd 0, 3
	xxsldwi 0, 0, 0, 1
	xscvspdpn 1, 0

As for the wording, how about something like this.

WARNING: This program is being run with address space layout randomization (ASLR) enabled which prevents the thread and memory sanitizers from working on powerpc64le. ASLR will be disabled and the program re-executed.

Rebased to account for Sanjay's patches.

This seems to have caused a build bot break on all PPC big endian bots. Sample build:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/20094

In D52900#1258162, @seurer wrote:

The similar change for aarch64 was done in

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cc

which seems a more "natural" location for this. Will that location not work for powerpc64?

I thought this needed to be done for more than just tsan on PPC. Lei can correct me if I'm wrong here.

Also, the wording of the warning is terrible (same as in the aarch64 one).

Feel free to suggest a different wording.

Since there seems to be overall agreement that this is the direction we want to go in, here's an update that removes the unsightly dual path for scalars and vectors.
Sorry about the delay in producing this update.

Also, please always publish Phabricator reviews with full context.

LGTM.

Rebased and added requested comment.

LGTM.

If the reviewers agree with my proposals for addressing the comments, I'll address them on the commit.

LGTM. Thanks for fixing this.

LGTM after Jinsong's comments are addressed.
+1 for the test case comments.

I'm just curious, is there something that runs after this pass that will clean up unreachable blocks if any became unreachable? For example, if MBB1 is the only predecessor of MBB2 and MBB2 is removed as a successor to MBB1, will something remove MBB2 afterwards.

@jedilyn Hi Ke Wen, thanks for your comments. This needs some cleanup with regard to which instructions match the semantics of F{MIN|MAX}NUM vs. F{MIN|MAX}NAN. I'll clean that up and re-post this for your review. Thanks.

Since there are unaddressed comments, I'm moving this off of my Must Review queue until they are addressed.

Please add a test case that we used to transform to a D-Form and no longer do with your patch (preferably one for DS-Form and one for DQ-Form).

Please provide the patch with full context as per http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface. Furthermore, if you would like to commit this, it'll have to go into trunk and then it can be merged into 7.0.1 (I'm not sure that we'd be able to merge it into any 6.0 release as there may not be a 6.0.2).
Finally, for the test case you can just compile anything with llc with the triple you are interested in and a CHECK directive can check for .abiversion 2 in the assembly.

LGTM.

LGTM.

LGTM. For a change such as this, I like the idea of committing a test case prior to the patch so the patch clearly shows the difference. Thank you.

Minor nit, but LGTM.

LGTM.

I think this patch caused failures (in addition to existing ones) on the PowerPC Big Endian buildbot (http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/23397). It fails unit tests in the file added in this patch, which is what makes me suspect that it is this patch that causes the failures. Can you please investigate or let me know if I need to do something on the buildbot (clean up directories, etc.)?

Hi Dean, I think this patch may have caused failures on the PPC Big Endian build bot and the failures were hidden by other build breaks that happened near the same time. Initially this caused failures at compile time which you fixed in r341042. However after that was fixed, we are now getting failures that I don't understand but certainly seem related to this patch (please refer to: http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/23322).
The failures being:

FAIL: LLVM-Unit::FDRTraceWriterTest.WriteToStringBufferVersion3
FAIL: LLVM-Unit::FDRTraceWriterTest.WriteToStringBufferVersion2
FAIL: LLVM-Unit::FDRTraceWriterTest.WriteToStringBufferVersion1