[llvm] e61bb1e - MachineConstantPool::getConstantPoolIndex - don't reuse mismatched constants contained undef/poison (Issue #63108)

[Simon Pilgrim via llvm-commits]

Author: Simon Pilgrim
Date: 2023-06-13T10:40:24+01:00
New Revision: e61bb1e541ffe3622712e3475c765f51ba88adfb

URL: https://github.com/llvm/llvm-project/commit/e61bb1e541ffe3622712e3475c765f51ba88adfb
DIFF: https://github.com/llvm/llvm-project/commit/e61bb1e541ffe3622712e3475c765f51ba88adfb.diff

LOG: MachineConstantPool::getConstantPoolIndex - don't reuse mismatched constants contained undef/poison (Issue #63108)

This patch fixes an issue where we were reusing constant pool entries that contained undef elements, despite the additional uses of the 'equivalent constant' requiring some/all of the elements to be zero.

The CanShareConstantPoolEntry helper function uses ConstantFoldCastOperand to bitcast the type mismatching constants to integer representations to allow comparison, but unfortunately this treats undef elements as zero (which they will be written out as in the final asm). This caused an issue where the original constant pool entry contained undef elements, which was shared with a later constant that required the elements to be zero. This then caused a later analysis pass to incorrectly discard these undef elements.

Ideally we need a more thorough analysis/merging of the constant pool entries so the elements are forced to real zero elements, but for now we just prevent reuse of the constant pool entry entirely if the constants don't have matching undef/poison elements.

Fixes #63108

Differential Revision: https://reviews.llvm.org/D152357

Added: 
    

Modified: 
    llvm/lib/CodeGen/MachineFunction.cpp
    llvm/test/CodeGen/X86/pr63108.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index e1cf41749608c..0f72e4b6108b1 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -1392,7 +1392,7 @@ MachineConstantPool::~MachineConstantPool() {
 }
 
 /// Test whether the given two constants can be allocated the same constant pool
-/// entry.
+/// entry referenced by \param A.
 static bool CanShareConstantPoolEntry(const Constant *A, const Constant *B,
                                       const DataLayout &DL) {
   // Handle the trivial case quickly.
@@ -1412,6 +1412,8 @@ static bool CanShareConstantPoolEntry(const Constant *A, const Constant *B,
   if (StoreSize != DL.getTypeStoreSize(B->getType()) || StoreSize > 128)
     return false;
 
+  bool ContainsUndefOrPoisonA = A->containsUndefOrPoisonElement();
+
   Type *IntTy = IntegerType::get(A->getContext(), StoreSize*8);
 
   // Try constant folding a bitcast of both instructions to an integer.  If we
@@ -1431,7 +1433,14 @@ static bool CanShareConstantPoolEntry(const Constant *A, const Constant *B,
     B = ConstantFoldCastOperand(Instruction::BitCast, const_cast<Constant *>(B),
                                 IntTy, DL);
 
-  return A == B;
+  if (A != B)
+    return false;
+
+  // Constants only safely match if A doesn't contain undef/poison.
+  // As we'll be reusing A, it doesn't matter if B contain undef/poison.
+  // TODO: Handle cases where A and B have the same undef/poison elements.
+  // TODO: Merge A and B with mismatching undef/poison elements.
+  return !ContainsUndefOrPoisonA;
 }
 
 /// Create a new entry in the constant pool or return an existing one.

diff  --git a/llvm/test/CodeGen/X86/pr63108.ll b/llvm/test/CodeGen/X86/pr63108.ll
index abdf6149e75b4..78ee1e1660ef3 100644
--- a/llvm/test/CodeGen/X86/pr63108.ll
+++ b/llvm/test/CodeGen/X86/pr63108.ll
@@ -4,8 +4,6 @@
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx2 | FileCheck %s --check-prefixes=AVX2
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mcpu=x86-64-v4 | FileCheck %s --check-prefixes=AVX512
 
-; FIXME: %vector.body.preheader <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u> constant should be <251,223,0,0,0,0,0,0,0,0,0,0,0,0,0,0>
-
 define i32 @PR63108() {
 ; SSE-LABEL: PR63108:
 ; SSE:       # %bb.0: # %entry
@@ -17,7 +15,7 @@ define i32 @PR63108() {
 ; SSE-NEXT:    jmp .LBB0_5
 ; SSE-NEXT:  .LBB0_2: # %vector.body.preheader
 ; SSE-NEXT:    pxor %xmm0, %xmm0
-; SSE-NEXT:    movdqa {{.*#+}} xmm2 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
+; SSE-NEXT:    movdqa {{.*#+}} xmm2 = [57339,0,0,0]
 ; SSE-NEXT:    xorl %eax, %eax
 ; SSE-NEXT:    .p2align 4, 0x90
 ; SSE-NEXT:  .LBB0_3: # %vector.body
@@ -51,7 +49,7 @@ define i32 @PR63108() {
 ; AVX1-NEXT:    vmovdqa {{.*#+}} xmm0 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
 ; AVX1-NEXT:    jmp .LBB0_5
 ; AVX1-NEXT:  .LBB0_2: # %vector.body.preheader
-; AVX1-NEXT:    vmovaps {{.*#+}} xmm0 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
+; AVX1-NEXT:    vmovaps {{.*#+}} xmm0 = [57339,0,0,0]
 ; AVX1-NEXT:    xorl %eax, %eax
 ; AVX1-NEXT:    .p2align 4, 0x90
 ; AVX1-NEXT:  .LBB0_3: # %vector.body
@@ -88,7 +86,7 @@ define i32 @PR63108() {
 ; AVX2-NEXT:    vmovdqa {{.*#+}} xmm0 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
 ; AVX2-NEXT:    jmp .LBB0_5
 ; AVX2-NEXT:  .LBB0_2: # %vector.body.preheader
-; AVX2-NEXT:    vmovdqa {{.*#+}} xmm0 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
+; AVX2-NEXT:    vmovdqa {{.*#+}} xmm0 = [57339,0,0,0]
 ; AVX2-NEXT:    xorl %eax, %eax
 ; AVX2-NEXT:    .p2align 4, 0x90
 ; AVX2-NEXT:  .LBB0_3: # %vector.body
@@ -125,7 +123,7 @@ define i32 @PR63108() {
 ; AVX512-NEXT:    vmovdqa {{.*#+}} xmm0 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
 ; AVX512-NEXT:    jmp .LBB0_5
 ; AVX512-NEXT:  .LBB0_2: # %vector.body.preheader
-; AVX512-NEXT:    vmovdqa {{.*#+}} xmm0 = <251,223,u,u,u,u,u,u,u,u,u,u,u,u,u,u>
+; AVX512-NEXT:    vmovdqa {{.*#+}} xmm0 = [57339,0,0,0]
 ; AVX512-NEXT:    xorl %eax, %eax
 ; AVX512-NEXT:    .p2align 4, 0x90
 ; AVX512-NEXT:  .LBB0_3: # %vector.body