[llvm] [llvm] Adding scalarization of `llvm.vector.insert` (PR #71614)

David Green via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 9 03:12:51 PST 2023


================
@@ -5891,8 +5910,11 @@ SDValue DAGTypeLegalizer::WidenVecRes_SETCC(SDNode *N) {
     InOp1 = GetWidenedVector(InOp1);
     InOp2 = GetWidenedVector(InOp2);
   } else {
-    InOp1 = DAG.WidenVector(InOp1, SDLoc(N));
-    InOp2 = DAG.WidenVector(InOp2, SDLoc(N));
+    do {
----------------
davemgreen wrote:

> Yes, but that's not what the AArch64 lowering wants (see https://github.com/llvm/llvm-project/blob/f0cdf4b468f6ee48b0d0d51ce78145455e2f07a6/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L24550).
> I believe the rational is v1i16 is not natively supported so we widen to the larger supported vector size.
> Changing that would fix this particular issue, but I believe it would create worse code in a bunch of cases.

Yeah, The point of that code is to act upon integer types, to keep them in vector registers as opposed to scalarizing them to gprs. A `fcmp uno <1 x half>` is really a floating point operation though, even if it's return type becomes a integer when it gets transformed to a `v1i16 setcc v1f16`. As an operation it still makes sense to scalarize it, not widen.

I agree that we shouldn't be changing the way that integer types are handled in general, but if there was a way to specify that a setcc with fp operands should really be treated like fp types, that would avoid a lot of the awkwardness here where we attempt to widen it only to scalarize again.

https://github.com/llvm/llvm-project/pull/71614


More information about the llvm-commits mailing list