[X86] Fix bug in masked compress

[aymanmus]

Summary

From the description of the x86 masked compress instruction:
"The opmask register k1 selects the active elements (partial vector or possibly non-contiguous if less than 16 active
elements) from the source operand to compress into a contiguous vector. The contiguous vector is written to the
destination starting from the low element of the destination operand.
Memory destination version: Only the contiguous vector is written to the destination memory location. EVEX.z
must be zero."
Which means that the instruction leaves the upper part of the memory untouched.
The current pattern assumes that the instruction pads the rest of the space with zeros and thus replace the select with ImmAllZeros with the masked compress instruction.

[delena]

test/CodeGen/X86/compress-maskz.ll

10

I don't see any store instruction in your check. Could you, please, use the "update_" utility to generate the full picture.

[igorb]

lib/Target/X86/X86ISelLowering.cpp

18879

Chain should be changed to LoadAddress

test/CodeGen/X86/compress-maskz.ll

1

could you please move the test to avx512vl-intrinsics.ll

[aymanmus]

• Fixed chains in load/store instructions
• Moved test to avx512vl-intrinsics.ll

[mkuper]

I'll let Igor/Elena review the logic, just a couple of comments on style.

lib/Target/X86/X86ISelLowering.cpp

18875

The formatting here looks wrong.

18880

Why is this a dyn_cast? if DAG.getLoad() is guaranteed to return a LoadSDNode (and I'd assume it is), you want a cast<>. If it's not, then you need to check the result of the dyn_cast.

[delena]

lib/Target/X86/X86ISelLowering.cpp

18874

We discussed with Igor and got into conclusion that this solution is not safe. The existing solution has the same problem.
If any of subsequent optimizations will break the store-vselect-compess sequence it may end up with memory exception.
In all other cases we handle masked store as a sole node in order to exclude this situation.

I suggest to add "IsCompressed" flag to MaskedStoreSDNode and than use it for COMPRESS_TO_MEM. Igor implemented masked_truncstore, you can take a look.

[aymanmus]

Entirely changed the approach of the solution as Elena suggested.
Masked compressed store is now lowered to a sole node of masked store with a new isCompressed flag.
Later on the node is replaced with the specific X86 avx3 compress machine instruction.

[delena]

include/llvm/CodeGen/SelectionDAGNodes.h

463 ↗

(On Diff #70704)

Please change it to IsCompressing. It will be compatible with IsTrucating.

lib/Target/X86/X86ISelLowering.cpp

18883

do not leave commented out code

[aymanmus]

• Removed commented out code.
• Changed "compressed" -> "compressing".

[delena]

include/llvm/CodeGen/SelectionDAG.h

972 ↗

(On Diff #72424)

wrong code alignment

include/llvm/CodeGen/SelectionDAGNodes.h

1974 ↗

(On Diff #72424)

Please add comments before the API. Please explain carefully what the compression means.

lib/Target/X86/X86ISelLowering.cpp

6

Code alignment

[aymanmus]

Fixed comment/

lib/Target/X86/X86ISelLowering.cpp

 //===-- X86ISelLowering.cpp - X86 DAG Lowering Implementation -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the interfaces that X86 uses to lower LLVM code into a
 // selection DAG.
 //
 //===----------------------------------------------------------------------===//
 
case COMPRESS_TO_MEM: {
 
     MemIntrinsicSDNode *MemIntr = dyn_cast<MemIntrinsicSDNode>(Op);
     assert(MemIntr && "Expected MemIntrinsicSDNode!");
 
     if (isAllOnesConstant(Mask)) // return just a store
       return DAG.getStore(Chain, dl, DataToCompress, Addr,
                           MemIntr->getMemOperand());
 
+    SDValue LoadAddress = DAG.getLoad(VT, SDLoc(Op), MemIntr->getChain(), Addr,
+                         MemIntr->getPointerInfo(), MemIntr->getAlignment());
     SDValue Compressed =
       getVectorMaskingNode(DAG.getNode(IntrData->Opc0, dl, VT, DataToCompress),
-                           Mask, DAG.getUNDEF(VT), Subtarget, DAG);
+                           Mask, LoadAddress, Subtarget, DAG);
     return DAG.getStore(Chain, dl, Compressed, Addr,
                         MemIntr->getMemOperand());
   }
   case TRUNCATE_TO_MEM_VI8:
   case TRUNCATE_TO_MEM_VI16:
   case TRUNCATE_TO_MEM_VI32: {
     SDValue Mask = Op.getOperand(4);
     SDValue DataToTruncate = Op.getOperand(3);
     SDValue Addr = Op.getOperand(2);
     SDValue Chain = Op.getOperand(0);
 
     MemIntrinsicSDNode *MemIntr = dyn_cast<MemIntrinsicSDNode>(Op);
     assert(MemIntr && "Expected MemIntrinsicSDNode!");

lib/Target/X86/X86InstrAVX512.td

def mr : AVX5128I<opc, MRMDestMem, (outs),
               (ins _.MemOp:$dst, _.RC:$src),
               OpcodeStr # "\t{$src, $dst|$dst, $src}",
               []>, EVEX_CD8<_.EltSize, CD8VT1>;
 
   def mrk : AVX5128I<opc, MRMDestMem, (outs),
               (ins _.MemOp:$dst, _.KRCWM:$mask, _.RC:$src),
               OpcodeStr # "\t{$src, $dst {${mask}}|$dst {${mask}}, $src}",
               [(store (_.VT (vselect _.KRCWM:$mask,
-                             (_.VT (X86compress  _.RC:$src)), _.ImmAllZerosV)),
+                             (_.VT (X86compress  _.RC:$src)), (_.VT (load addr:$dst)))),
                 addr:$dst)]>,
               EVEX_K, EVEX_CD8<_.EltSize, CD8VT1>;
 }
 
 multiclass compress_by_elt_width<bits<8> opc, string OpcodeStr,
                                  AVX512VLVectorVTInfo VTInfo> {
   defm Z : compress_by_vec_width<opc, VTInfo.info512, OpcodeStr>, EVEX_V512;
 

test/CodeGen/X86/compress-maskz.ll

+;RUN: llc < %s -O0 -mattr=+avx512vl -mtriple=x86_64-unknown | FileCheck %s --check-prefix=CHECK
+
+@xmm = common global <4 x i32> zeroinitializer, align 16
+@k8 = common global i8 0, align 1
+@res = common global <4 x i32> zeroinitializer, align 16
+
+define i32 @main() {
+; CHECK-LABEL: main:
+; CHECK:    vmovdqa32 {{.*}}(%rip), %xmm0
+; CHECK:    vpcompressd %xmm0, %xmm0 {%k1} {z}
+entry:
+  %.compoundliteral = alloca <2 x i64>, align 16
+  %res = alloca <4 x i32>, align 16
+  %a0 = load <4 x i32>, <4 x i32>* @xmm, align 16
+  %a2 = load i8, i8* @k8, align 1
+  %a21 = call <4 x i32> @llvm.x86.avx512.mask.compress.d.128(<4 x i32> %a0, <4 x i32> zeroinitializer, i8 %a2) #2
+  store volatile <4 x i32> %a21, <4 x i32>* %res, align 16
+  store <2 x i64> zeroinitializer, <2 x i64>* %.compoundliteral, align 16
+  ret i32 0
+}
+
+declare <4 x i32> @llvm.x86.avx512.mask.compress.d.128(<4 x i32>, <4 x i32>, i8)