[PowerPC] Support constraint code "ww"

[MaskRay]

Summary

"ww" and "ws" are both constraint codes for VSX vector registers that
holds scalar double data. "ww" is preferred for float while "ws" is
preferred for double.

[jsji]

It is great to add ww for compatibility.
However if we are going to add ww, looks like we should update ws as well?

clang/lib/Basic/Targets/PPC.h

211 ↗

(On Diff #207707)

Add some more comments for w to distinguish it from s?

Do we want to keep compatibility with GCC?
According to https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Machine-Constraints.html#Machine-Constraints,
ww is FP or VSX register to perform float operations under -mvsx or NO_REGS.,
while ws is VSX vector register to hold scalar double values .

So ww can use FP while ws can NOT ?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

14080 ↗

(On Diff #207707)

Should we exclude FP for ws and return VFRCRegClass instead of VSFRCRegClass ?

llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll

42 ↗

(On Diff #207707)

Maybe we should add another test for ws as well? The above test is actually for 'x' modifier?

[MaskRay]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

14080 ↗

(On Diff #207707)

Can you elaborate on what I should do? I'm not familiar with the register info stuff...

[MaskRay]

llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll

42 ↗

(On Diff #207707)

I think it is incorrect if the 'x' modifier is not used, so we probably don't have to check the no-modifier case.

[MaskRay]

clang/lib/Basic/Targets/PPC.h

211 ↗

(On Diff #207707)

I played with "ws" and "ww" but can't find any behavior difference from assembly produced by powerpc64le-linux-gnu-gcc. I'll keep the current form (which is known to make musl fmax/fmaxf build) unless the gcc semantics are clearer.

[jsji]

LGTM. Thanks for investigating GCC behavior.

clang/lib/Basic/Targets/PPC.h

211 ↗

(On Diff #207707)

OK. Thanks. So maybe it is just the misleading doc problem of GCC.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

14080 ↗

(On Diff #207707)

VSFRC contains both F8RC and VFRC. F8RC is FP.
So if ws can NOT use FP, then we should not use VSFRC.

However, if ws can use FP as well as you found in later GCC experiments, then we don't need to do this.

[MaskRay]

float ws_float(float x, float y) {
  __asm__ ("xsadddp %x0, %x1, %x2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
float ww_float(float x, float y) {
  __asm__ ("xsadddp %x0, %x1, %x2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}

double ws_double(double x, double y) {
  __asm__ ("xsadddp %x0, %x1, %x2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
double ww_double(double x, double y) {
  __asm__ ("xsadddp %x0, %x1, %x2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}

% powerpc64le-linux-gnu-gcc -O2 a.c -S -o - | grep xsadd
        xsadddp 1, 1, 2
        xsadddp 1, 1, 2
        xsadddp 1, 1, 2
        xsadddp 1, 1, 2
% clang -target ppc64le -O2 a.c -S -o - | grep xsadd
# same output

float scalar_ww_float(float x, float y) {
  __asm__ ("fadds %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}
float scalar_ws_float(float x, float y) {
  __asm__ ("fadds %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
double scalar_ww_double(double x, double y) {
  __asm__ ("fadds %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}
double scalar_ws_double(double x, double y) {
  __asm__ ("fadds %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}

% powerpc64le-linux-gnu-gcc -O2 a.c -S -o - | grep fadds
        fadds 1, 1, 2
        fadds 1, 1, 2
        fadds 1, 1, 2
        fadds 1, 1, 2
% clang -target ppc64le -O2 a.c -S -o - | grep fadds
# same output

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

14080 ↗

(On Diff #207707)

float ws_float(float x, float y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
float ww_float(float x, float y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}

double ws_double(double x, double y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
double ww_double(double x, double y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}

% powerpc64le-linux-gnu-gcc -O2 a.c -S -o - | grep xsadd
        xsadddp 1, 1, 2
        xsadddp 1, 1, 2
        xsadddp 1, 1, 2
        xsadddp 1, 1, 2

cfe/trunk/lib/Basic/Targets/PPC.h

case 'v': // Altivec vector register
       if (FloatABI == SoftFloat)
         return false;
       Info.setAllowsRegister();
       break;
     case 'w':
       switch (Name[1]) {
       case 'd': // VSX vector register to hold vector double data
       case 'f': // VSX vector register to hold vector float data
-      case 's': // VSX vector register to hold scalar float data
+      case 's': // VSX vector register to hold scalar double data
+      case 'w': // VSX vector register to hold scalar double data
       case 'a': // Any VSX register
       case 'c': // An individual CR bit
       case 'i': // FP or VSX register to hold 64-bit integers data
         break;
       default:
         return false;
       }
       Info.setAllowsRegister();

cfe/trunk/test/CodeGen/ppc64-inline-asm.c

 unsigned char test_wc_i8(unsigned char b1, unsigned char b2) {
   unsigned char o;
   asm("crand %0, %1, %2" : "=wc"(o) : "wc"(b1), "wc"(b2) : );
   return o;
 // CHECK-LABEL: zeroext i8 @test_wc_i8(i8 zeroext %b1, i8 zeroext %b2)
 // CHECK: call i8 asm "crand $0, $1, $2", "=^wc,^wc,^wc"(i8 %b1, i8 %b2)
 }
 
+float test_fmaxf(float x, float y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ww"(x) : "ww"(x), "ww"(y));
+  return x;
+// CHECK-LABEL: float @test_fmaxf(float %x, float %y)
+// CHECK: call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ww,^ww,^ww"(float %x, float %y)
+}
+
+double test_fmax(double x, double y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ws"(x) : "ws"(x), "ws"(y));
+  return x;
+// CHECK-LABEL: double @test_fmax(double %x, double %y)
+// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ws,^ws,^ws"(double %x, double %y)
+}

llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp

case 'Z':
       // and forming the complete address in the second register. This is
       // suboptimal.
       return C_Memory;
     }
   } else if (Constraint == "wc") { // individual CR bits.
     return C_RegisterClass;
   } else if (Constraint == "wa" || Constraint == "wd" ||
              Constraint == "wf" || Constraint == "ws" ||
-             Constraint == "wi") {
+             Constraint == "wi" || Constraint == "ww") {
     return C_RegisterClass; // VSX registers.
   }
   return TargetLowering::getConstraintType(Constraint);
 }
 
 /// Examine constraint type and operand type and determine a weight value.
 /// This object must already have been set up with the operand type
 /// and the current alternative constraint selected.
PPCTargetLowering::getSingleConstraintMatchWeight(
   // Look at the constraint type.
   if (StringRef(constraint) == "wc" && type->isIntegerTy(1))
     return CW_Register; // an individual CR bit.
   else if ((StringRef(constraint) == "wa" ||
             StringRef(constraint) == "wd" ||
             StringRef(constraint) == "wf") &&
            type->isVectorTy())
     return CW_Register;
-  else if (StringRef(constraint) == "ws" && type->isDoubleTy())
-    return CW_Register;
   else if (StringRef(constraint) == "wi" && type->isIntegerTy(64))
     return CW_Register; // just hold 64-bit integers data.
+  else if (StringRef(constraint) == "ws" && type->isDoubleTy())
+    return CW_Register;
+  else if (StringRef(constraint) == "ww" && type->isFloatTy())
+    return CW_Register;
 
   switch (*constraint) {
   default:
     weight = TargetLowering::getSingleConstraintMatchWeight(info, constraint);
     break;
   case 'b':
     if (type->isIntegerTy())
       weight = CW_Register;
if (Constraint.size() == 1) {
     }
   } else if (Constraint == "wc" && Subtarget.useCRBits()) {
     // An individual CR bit.
     return std::make_pair(0U, &PPC::CRBITRCRegClass);
   } else if ((Constraint == "wa" || Constraint == "wd" ||
              Constraint == "wf" || Constraint == "wi") &&
              Subtarget.hasVSX()) {
     return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) {
     if (VT == MVT::f32 && Subtarget.hasP8Vector())
       return std::make_pair(0U, &PPC::VSSRCRegClass);
     else
       return std::make_pair(0U, &PPC::VSFRCRegClass);
   }
 
   std::pair<unsigned, const TargetRegisterClass *> R =
       TargetLowering::getRegForInlineAsmConstraint(TRI, Constraint, VT);

llvm/trunk/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll

define double @test() {
   entry:
     %0 = tail call double asm "mtvsrd ${0:x}, 1", "=^ws,~{f0},~{f1},~{f2},~{f3},~{f4},~{f5},~{f6},~{f7},~{f8},~{f9},~{f10},~{f11},~{f12},~{f13},~{f14}"()
     ret double %0
 ; CHECK-LABEL: test:
 ; CHECK: #APP
 ; CHECK: mtvsrd v2, r1
 ; CHECK: #NO_APP
 }
+
+define float @test_ww(float %x, float %y) {
+  %1 = tail call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ww,^ww,^ww"(float %x, float %y)
+  ret float %1
+; CHECK-LABEL: test_ww:
+; CHECK: #APP
+; CHECK: xsmaxdp f1, f1, f2
+; CHECK: #NO_APP
+}

llvm/trunk/test/CodeGen/PowerPC/vec-asm-disabled.ll

 entry:
   %0 = tail call { i32, <4 x float> } asm "xxsldwi ${1:x},${2:x},${2:x},3", "=^wi,=&^wi,^wi"(<4 x float> %__A) #0
   %asmresult = extractvalue { i32, <4 x float> } %0, 0
   ret i32 %asmresult
 
 ; CHECK: error: couldn't allocate output register for constraint 'wi'
 }
 
+define float @test_ww(float %x, float %y) #0 {
+  %1 = tail call float asm "xsmaxdp ${0:x},${1:x},${2:x}", "=^ww,^ww,^ww"(float %x, float %y) #0
+  ret float %1
+; CHECK: error: couldn't allocate output register for constraint 'ww'
+}
+
+define double @test_ws(double %x, double %y) #0 {
+  %1 = tail call double asm "xsmaxdp ${0:x},${1:x},${2:x}", "=^ws,^ws,^ws"(double %x, double %y) #0
+  ret double %1
+; CHECK: error: couldn't allocate output register for constraint 'ws'
+}
+
 attributes #0 = { nounwind "target-features"="-vsx" }