[llvm] [DirectX] Undo SimplifyCFG by reversing select into if else basic blocks (PR #153858)

[Farzon Lotfi via llvm-commits]

farzonl wrote:

> > > Is there a general constraint that `target("dx.RawBuffer")` cannot be used in select or just with these intrinsics?
> > > I'm wondering if we should be preventing this from happening in the first place if it's a legality constraint. We have
> > > https://github.com/llvm/llvm-project/blob/f0967fca04c880e9aabd5be043a85127faabb4c6/llvm/lib/Transforms/Utils/Local.cpp#L3839
> > > 
> > > to control this and could come up with a way to support target types or target intrinsics in there.
> > 
> > 
> > This is an interesting idea. Are you thinking that we'd do something like add another property to `TargetExtType::hasProperty` so that target types could opt in/out of `canReplaceOperandWithVariable` (or possibly more generally say it can't be replaced with a non-constant value)?
> 
> Depends on what the exact requirements are. I think the options here would be:
> 
> 1. Add a TargetExtType property that forbids use of the type in select/phi (and presumably non-intrinsic calls?) Basically, to give the target type the same constraints as a `token` type.
> 2. Add an attribute similar to `immarg` which can be used to prevent introduction of select/phi for an intrinsic argument -- but without limiting to constant integer arguments.

I'm not sure this is going to work out for us.
CC: @bogner 

So didn't want to invest all the plumbing to get this working just for RawBuffers so keeping the catch all broad for now. Net result was this experiment 
```diff
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index ac344904f90f..c0995bf6346e 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3852,6 +3852,9 @@ bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
   if (I->isLifetimeStartOrEnd())
     return false;
 
+  if (Op->getType()->isTargetExtTy())
+    return false;
+
   // Early exit.
   if (!isa<Constant, InlineAsm>(Op))
     return true;
```

This does prevent the select from happening
```llvm
define void @CSMain() local_unnamed_addr #1 {
entry:
  %d.cb_h.i.i = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_d, 16, 0, 4, 8, 12)) @llvm.dx.resource.handlefromimplicitbinding.tdx.CBuffer_tdx.Layout_s___cblayout_ds_16_0_4_8_12tt(i32 3, i32 0, i32 1, i32 0, i1 false, ptr nonnull @d.str)
  store target("dx.CBuffer", target("dx.Layout", %__cblayout_d, 16, 0, 4, 8, 12)) %d.cb_h.i.i, ptr @d.cb, align 4
  %0 = load i32, ptr addrspace(2) @h, align 4, !tbaa !6
  %tobool.not.i = icmp eq i32 %0, 0
  %1 = load i32, ptr addrspace(2) @g, align 4, !tbaa !6
  br i1 %tobool.not.i, label %if.else.i, label %if.then.i

if.then.i:                                        ; preds = %entry
  %2 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 1, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str.2)
  %3 = load i32, ptr addrspace(2) @f, align 4, !tbaa !6
  %4 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %2, i32 %3)
  br label %_Z6CSMainv.exit

if.else.i:                                        ; preds = %entry
  %5 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 0, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
  %6 = load i32, ptr addrspace(2) @e, align 4, !tbaa !6
  %7 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %5, i32 %6)
  br label %_Z6CSMainv.exit

_Z6CSMainv.exit:                                  ; preds = %if.then.i, %if.else.i
  %.sink1 = phi ptr [ %4, %if.then.i ], [ %7, %if.else.i ]
  %8 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 2, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str.4)
  %9 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %8, i32 %1)
  %10 = load half, ptr %.sink1, align 2, !tbaa !10
  store half %10, ptr %9, align 2, !tbaa !10
  ret void
}
```
But now we are blowing up in `DXILResourceAccess.cpp:234` instead of  `DXILOpLowering.cpp`
https://github.com/llvm/llvm-project/blob/b35b6297fd5193e10e0c57d3371f7326fbd40ae3/llvm/lib/Target/DirectX/DXILResourceAccess.cpp#L234

What is happening is `replaceAccess` is only designed to support loads, stores, and geps, and  now we are introducing a phi node

```gdb
lldb) expr II->dump()
  %4 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %2, i32 %3)
(lldb) expr Current.Access->dump()
  %.sink1 = phi ptr [ %4, %if.then.i ], [ %7, %if.else.i ]
```

This is actually a similar failure we were having  when we were just trying to put the call instructions into  the if and else basic blocks. To fix this issue the whole chain from load to store needs to be seperate. 


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