[PATCH] D151848: [X86, Peephole] Enable FoldImmediate for X86

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/PeepholeOptimizer.cpp:1391
+      // FoldImmediate can delete ImmDefMI if MI was its only user. If ImmDefMI
+      // is not deleted, and we happenly get a same MI, we can delete MI and
+      // replace its users.
----------------
happenly?


================
Comment at: llvm/test/CodeGen/X86/popcnt.ll:619
+; X86-NEXT:    movl %eax, %ecx
+; X86-NEXT:    andl $858993459, %ecx # imm = 0x33333333
 ; X86-NEXT:    shrl $2, %eax
----------------
This defeats the code size optimization in SelectionDAG.  See X86InstrInfo.td

```
// If we have multiple users of an immediate, it's much smaller to reuse         
// the register, rather than encode the immediate in every instruction.          
// This has the risk of increasing register pressure from stretched live         
// ranges, however, the immediates should be trivial to rematerialize by         
// the RA in the event of high register pressure.                                
// TODO : This is currently enabled for stores and binary ops. There are more    
// cases for which this can be enabled, though this catches the bulk of the      
// issues.                                                                       
// TODO2 : This should really also be enabled under O2, but there's currently    
// an issue with RA where we don't pull the constants into their users           
// when we rematerialize them. I'll follow-up on enabling O2 after we fix that   
// issue.                                                                        
// TODO3 : This is currently limited to single basic blocks (DAG creation        
// pulls block immediates to the top and merges them if necessary).              
// Eventually, it would be nice to allow ConstantHoisting to merge constants     
// globally for potentially added savings.                                       
//                                                                               
def imm_su : PatLeaf<(imm), [{                                                   
    return !shouldAvoidImmediateInstFormsForSize(N);                             
}]>;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151848