[PATCH] Add VSX Scalar loads and stores to the PPC back end

Nemanja Ivanovic nemanja.i.ibm at gmail.com
Tue May 5 05:08:30 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:2714
@@ -2713,2 +2713,3 @@
     else if (N->getValueType(0) == MVT::f32)
-      SelectCCOp = PPC::SELECT_CC_F4;
+      if (PPCSubTarget->hasVSX() && PPCSubTarget->hasP8Vector())
+        SelectCCOp = PPC::SELECT_CC_VSSRC;
----------------
kbarton wrote:
> You can drop the hasVSX() from this check - it is a prerequisite for hasP8Vector (see FeatureP8Vector definition in PPC.td)
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.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:100
@@ +99,3 @@
+
+    // VSX scalar loads introduced in ISA 2.07
+    let Predicates = [HasP8Vector] in {
----------------
kbarton wrote:
> One general comment, more organizational then anything else. In PPCInstrAltivec.td, 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?
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?

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1033
@@ -972,3 +1032,3 @@
 
-let Predicates = [HasDirectMove, HasVSX] in {
+let Predicates = [HasVSX] in {
 // VSX direct move instructions
----------------
wschmidt wrote:
> nemanjai wrote:
> > 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.
> 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.
Noted. I will put it back in.

================
Comment at: test/CodeGen/PowerPC/ppc64le-smallarg.ll:47
@@ -46,2 +46,3 @@
+; CHECK: lxsspx {{[0-9]+}}, {{[0-9]+}}, [[TOCREG]]
 ; CHECK: blr
 
----------------
wschmidt wrote:
> You don't have a run line specifying -mcpu, so this is potentially fragile.
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.

http://reviews.llvm.org/D9440

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list