[PATCH] D136472: [DAGCombiner][RISCV] Make foldBinOpIntoSelect work correctly with opaque constants.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2288
+    if (BinOpcode == ISD::AND && isNullOrNullSplat(CT))
+      NewCT = DAG.getConstant(0, DL, VT);
+    else if (BinOpcode == ISD::OR && isAllOnesOrAllOnesSplat(CT))
----------------
spatel wrote:
> craig.topper wrote:
> > spatel wrote:
> > > Add a note here or to the text above to explain that we don't re-use an opaque 0/1 because that could inf-loop.
> > The 0/-1 weren’t opaque. I was just being paranoid about using 0/-1 that might be mixed with undef. Do I need to worry about that here?
> Ah, I misunderstood how this was looping. 
> Still worth a note that we're creating 0/-1 directly here rather than relying on getNode because CBO could be opaque?
For posterity, we loop between these two states

```
Combining: t15: i32 = or t13, OpaqueConstant:i32<2048>                           
Creating new node: t23: i32 = or Constant:i32<-1>, OpaqueConstant:i32<2048>      
Creating new node: t24: i32 = or Constant:i32<0>, OpaqueConstant:i32<2048>       
Creating new node: t25: i32 = select t7, t23, t24                                
 ... into: t25: i32 = select t7, t23, t24                                        
                                                                                 
Combining: t19: i64 = any_extend t25                                             
                                                                                 
Combining: t25: i32 = select t7, t23, t24                                        
Creating new node: t26: i32 = select t7, Constant:i32<-1>, Constant:i32<0>       
Creating new node: t27: i32 = or t26, OpaqueConstant:i32<2048>                   
 ... into: t27: i32 = or t26, OpaqueConstant:i32<2048>
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136472/new/

https://reviews.llvm.org/D136472