[llvm] [InferAddressSpaces] Fix null PHI operand when an incoming flat expression is not rewritten (PR #205749)

[Robert Imschweiler via llvm-commits]

================
@@ -1550,9 +1557,19 @@ bool InferAddressSpacesImpl::rewriteWithNewAddressSpaces(
 
     unsigned OperandNo = PoisonUse->getOperandNo();
     assert(isa<PoisonValue>(NewV->getOperand(OperandNo)));
-    WeakTrackingVH NewOp = ValueWithNewAddrSpace.lookup(PoisonUse->get());
-    assert(NewOp &&
-           "poison replacements in ValueWithNewAddrSpace shouldn't be null");
+    Value *NewOp = ValueWithNewAddrSpace.lookup(PoisonUse->get());
+    if (!NewOp) {
+      // The operand was never rewritten into the new address space (e.g. a flat
----------------
ro-i wrote:

can you please give a bit more context about how this situation occurs and why we end up removing the assert? Even looking at your new IR reproducers, I'm not sure I really understand what the issue is.

If I run the pass on phi_incoming_instruction with -print-before-all and -debug, I get
```
; *** IR Dump Before InferAddressSpacesPass on phi_incoming_instruction ***
define void @phi_incoming_instruction(ptr addrspace(1) %arg, i1 %c) {
entry:
  %arg.cast = addrspacecast ptr addrspace(1) %arg to ptr
  br i1 %c, label %merge, label %loop

loop:                                             ; preds = %loop, %entry
  %x = phi ptr [ poison, %entry ], [ %xn, %loop ]
  %xn = getelementptr i8, ptr %x, i64 1
  br i1 %c, label %loop, label %merge

merge:                                            ; preds = %loop, %entry
  %p = phi ptr [ %arg.cast, %entry ], [ %xn, %loop ]
  %v = load i32, ptr %p, align 4
  ret void
}

Features:+load-store-opt,+enable-ds128,+flat-for-global,+unaligned-access-mode,+trap-handler,+enable-prt-strict-null,
CPU:generic-hsa
TuneCPU:generic-hsa

xnack setting for subtarget: Unsupported
sramecc setting for subtarget: Unsupported
Updating the address space of
    %x = phi ptr [ poison, %entry ], [ %xn, %loop ]
Updating the address space of
    %p = phi ptr [ %arg.cast, %entry ], [ %xn, %loop ]
Updating the address space of
    %arg.cast = addrspacecast ptr addrspace(1) %arg to ptr
  to 1
Updating the address space of
    %p = phi ptr [ %arg.cast, %entry ], [ %xn, %loop ]
  to 1
Updating the address space of
    %xn = getelementptr i8, ptr %x, i64 1
opt: /work1/omp-nightly/build/git/aomp23.0/llvm-project/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:1565: bool {anonymous}::InferAddressSpacesImpl::rewriteWithNewAddressSpaces(llvm::ArrayRef<llvm::WeakTrackingVH>, const ValueToAddrSpaceMapTy&, const PredicatedAddrSpaceMapTy&) const: Assertion `NewOp && "poison replacements in ValueWithNewAddrSpace shouldn't be null"' failed.
```

Can you elaborate on the issue and what the *root* cause is? Maybe I'm missing something, but I wonder if we're trying to fix this on the wrong level / the wrong stage of the pass

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