Page MenuHomePhabricator

Add VSX Scalar loads and stores to the PPC back end

Authored by nemanjai on May 1 2015, 11:48 AM.



This patch does two things:

  1. Adds the new VSX scalar loads and stores introduced in ISA 2.07
  2. Defines a new register class (VSSRC) similar to VSFRC that allows single precision (f32) values to be operated on in VSX registers.

In ISA 2.07, there is more complete support for single precision operations on the full set of VSX registers, so the new register class is needed.

Diff Detail


Event Timeline

nemanjai updated this revision to Diff 24823.May 1 2015, 11:48 AM
nemanjai retitled this revision from to Add VSX Scalar loads and stores to the PPC back end.
nemanjai updated this object.
nemanjai edited the test plan for this revision. (Show Details)
nemanjai added reviewers: wschmidt, hfinkel, kbarton, seurer.
nemanjai set the repository for this revision to rL LLVM.
nemanjai added a subscriber: Unknown Object (MLST).
nemanjai added inline comments.May 1 2015, 11:52 AM

There is no semantic difference, but for consistency, this will be updated to replace xaddr with xoaddr.


This was removed in response to a comment to the commit for the "direct moves" patch. The requirement is redundant since the DAG nodes only come from the custom lowering code and that code is guarded for direct moves.

wschmidt edited edge metadata.May 4 2015, 7:21 AM

In general this looks very good. Just a few inline comments from me. I haven't added a register class myself, so you may want a thumbs-up from someone with that experience, but I can't think offhand of anything you might have missed.


This is a harmless redundancy and to me it seems worthwhile to leave in place. Code gets re-designed, and it may not be a redundant requirement in the future.


&& Subtarget.hasVSX() ?


You don't have a run line specifying -mcpu, so this is potentially fragile.

kbarton added inline comments.May 4 2015, 1:09 PM

You can drop the hasVSX() from this check - it is a prerequisite for hasP8Vector (see FeatureP8Vector definition in


Same here - hasVSX() can be dropped from the check


Here as well - no hasVSX() needed.


One general comment, more organizational then anything else. In, we chose to put all the P8 instructions together, at the end of the file, instead of intermixing them with existing instructions. Do we want to do the same thing in this file as well, if for no other reason then to be consistent?


nope, shouldn't be needed here :)

nemanjai added inline comments.May 5 2015, 5:08 AM

I was under the impression that specifying -mattr=-vsx would not automatically turn off power8-vector, but you are right it does. I'll remove the redundant checks. Unless anyone has any objections to this.


It is all the same to me. I just put them here because the loads/stores are here. I can just as easily move them. Are we all in agreement that this is what we want?


Noted. I will put it back in.


I can certainly put it in, but I avoided doing that for 2 reasons:

  1. There is metadata in the test case identifying the target triple (which LLC respects) so the features are getting picked up due to the powerpc64le arch. This makes it work on other platforms too (tried on X86).
  2. Since I didn't write the test case, I am not sure if the intent indeed was to not specify the -mcpu for some reason but rather provide the triple in the IR metadata.

Please confirm that you would like me to remove the

target triple = "powerpc64le-unknown-linux-gnu"

and add -mcpu=pwr8. I can then put back the original checks for other CPU's.

wschmidt added inline comments.May 5 2015, 5:35 AM

I agree with Kit; let's try to keep the P8Vector stuff together.


No, after reading your comments I'm okay with leaving it as is. Relying on the arch for POWER8 is ok, but bear in mind that instructions for any follow-on ISA in the future won't be able to do this.

hfinkel edited edge metadata.May 5 2015, 6:15 PM

Modulo comments already made by others, this LGTM. I assume you'll be adding the other f32 VSX instructions in the near future.

nemanjai updated this revision to Diff 25171.May 7 2015, 6:46 AM
nemanjai edited edge metadata.

Addressed the review comments.

  • Removed redundant checks for VSX
  • Moved all the P8Vector loads and stores down to the bottom of
  • Put the check for Direct Moves back in (for clarity and possible future needs)

VSX scalar arithmetic instructions to follow.

wschmidt accepted this revision.May 7 2015, 7:31 AM
wschmidt edited edge metadata.

LGTM, thanks!

This revision is now accepted and ready to land.May 7 2015, 7:31 AM
nemanjai closed this revision.May 7 2015, 11:27 AM

Committed revision 236755.