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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon created this revision.
RKSimon added reviewers: nikic, efriedma, foad.
Herald added subscribers: StephenFan, pengfei, arphaman, hiraditya.
Herald added a project: All.
RKSimon requested review of this revision.
Herald added a project: LLVM.

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 bitcasts 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 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.

Fixes #63108


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152357

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


Index: llvm/test/CodeGen/X86/pr63108.ll
===================================================================
--- llvm/test/CodeGen/X86/pr63108.ll
+++ 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 @@
 ; 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 @@
 ; 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 @@
 ; 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 @@
 ; 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
Index: llvm/lib/CodeGen/MachineFunction.cpp
===================================================================
--- llvm/lib/CodeGen/MachineFunction.cpp
+++ llvm/lib/CodeGen/MachineFunction.cpp
@@ -1392,7 +1392,7 @@
 }
 
 /// 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 @@
   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,13 @@
     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.
+  // 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152357.529233.patch
Type: text/x-patch
Size: 3629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230607/26645220/attachment.bin>