[PATCH] D16137: AVX512: VMOVDQU8/16/32/64 (load) intrinsic implementation.

[Mitch Bodart via llvm-commits]

mbodart added a comment.

I'm not sure I understand why this is just now becoming an issue.

Is the need for an X86-specific override of LowerOperationWrapper driven by an existing problem
with SINT_TO_FP lowering, or by a problem that is only exposed when adding the new masked load intrinsics?

Is the chain being dropped for both SINT_TO_FP and masked loads, or just one of them.

What are the safety consequences of dropping the chain, wrt losing an ordering dependence?

And why is I64 write mask legalization fine for most masked intrinsics, but not the masked loads?
Is it because none of the other existing I64-masked intrinsics produce an additional chain result?

A concrete example or two, showing the DAG snippets during legalization , would be helpful.

- mitch


================
Comment at: lib/Target/X86/X86ISelLowering.h:689
@@ +688,3 @@
+                                       SmallVectorImpl<SDValue> &Results,
+                                       SelectionDAG &DAG) const;
+
----------------
Please add the "override" indicator.

================
Comment at: lib/Target/X86/X86InstrAVX512.td:2753
@@ -2752,11 +2752,3 @@
                                  HasAVX512>, XS, VEX_W, EVEX_CD8<64, CD8VF>;
 
 def: Pat<(int_x86_avx512_mask_storeu_d_512 addr:$ptr, (v16i32 VR512:$src),
----------------
Can you please explain why these patterns are being deleted?

================
Comment at: test/CodeGen/X86/avx512-intrinsics.ll:6643-6644
@@ +6642,4 @@
+; CHECK-NEXT:    vmovdqu32 (%rdi), %zmm0
+; CHECK-NEXT:    vmovdqu32 (%rdi), %zmm0 {%k1}
+; CHECK-NEXT:    vmovdqu32 (%rdi), %zmm1 {%k1} {z}
+; CHECK-NEXT:    vpaddd %zmm0, %zmm1, %zmm0
----------------
This kind of test is OK for now.
But note that the second vmovdqu32 is completely redundant.
And the last vmovdqu32's source operand could be replaced by %zmm0.
So the test may need maintenance as optimizations improve.

This same pattern is used in many test functions in these test files,
and of course they will all have the same issue.



Repository:
  rL LLVM

http://reviews.llvm.org/D16137