[llvm] [LLVM][DAGCombiner] Look through freeze when combining extensions of loads (PR #175022)

[David Sherwood via llvm-commits]

david-arm wrote:

Hi @RKSimon @sdesmalen-arm, thanks for your comments. I thought it was easier to address all the comments here since there was some overlap between them.

In this latest version I have added support for cases where we have freeze(load(..)) and there are multiple uses due to the chain only. This permits optimisation of more cases as you can see with the last commit. I've also used @RKSimon's suggestion of adding AssertSext and AssertZext to aid optimisations, although it required adding new splitting legalisation code for AssertSext.

I tried adding support for multiple uses of the freeze itself, but ran into various issues:

For scalar types I hit a lowering issue for test knownbits_zext_in_reg in llvm/test/CodeGen/X86/known-bits.ll that I couldn't see any obvious way to resolve in generic code. Without the optimisation the final DAG looks like:

```
Optimized legalized selection DAG: %bb.0 'knownbits_zext_in_reg:BB'
SelectionDAG has 21 nodes:
  t0: ch,glue = EntryToken
        t3: i32,ch = load<(load (s32) from %fixed-stack.0)> t0, FrameIndex:i32<-1>, undef:i32
      t5: i8,ch = load<(load (s8) from %ir.0)> t0, t3, undef:i32
    t8: i8 = freeze t5
  t70: i32 = zero_extend t8
          t76: i32 = mul t70, Constant:i32<101>
        t66: i32 = srl t76, Constant:i8<14>
      t69: i8 = truncate t66
    t12: ch = CopyToReg t0, Register:i8 %0, t69
          t72: i32 = mul t70, Constant:i32<177>
        t58: i32 = srl t72, Constant:i8<14>
      t61: i8 = truncate t58
    t16: ch = CopyToReg t0, Register:i8 %1, t61
  t17: ch = TokenFactor t12, t16
```

and with the optimisation it looks like this:

```
Optimized legalized selection DAG: %bb.0 'knownbits_zext_in_reg:BB'
SelectionDAG has 23 nodes:
  t0: ch,glue = EntryToken
          t3: i32,ch = load<(load (s32) from %fixed-stack.0)> t0, FrameIndex:i32<-1>, undef:i32
        t33: i16,ch = load<(load (s8) from %ir.0), zext from i8> t0, t3, undef:i32
      t34: i16 = freeze t33
    t77: i32 = zero_extend t34
  t73: i32 = AssertZext t77, ValueType:ch:i8
          t83: i32 = mul t73, Constant:i32<101>
        t66: i32 = srl t83, Constant:i8<14>
      t69: i8 = truncate t66
    t12: ch = CopyToReg t0, Register:i8 %0, t69
          t75: i32 = mul t73, Constant:i32<177>
        t59: i32 = srl t75, Constant:i8<14>
      t62: i8 = truncate t59
    t16: ch = CopyToReg t0, Register:i8 %1, t62
  t17: ch = TokenFactor t12, t16
```

which leads to the generation of an extra mov instruction. It's not obvious to me how best to fix this - is this just an isel issue for X86 or does there need to be a DAG combine that rewrites extending load to be i8 -> i32? Also, in all other scalar tests where this made a difference it didn't actually reduce instruction count and I couldn't really see any benefit. So, I then tried restricting this multiple-use case to vectors only but I hit another roadblock. The function ExtendUsesToFormExtLoad asks the target if truncates are free, which at least for AArch64 is not true for vectors so we immediately bail out. So at least for AArch64 I couldn't write test cases to expose this issue and we need to have a target that says "vector truncates are free" and "extending vector loads are legal/desirable". Furthermore, if one of the uses is a setcc the code seems to assume it will only ever be a scalar due to the check for ConstantSDNode.

For these reason I decided to continue limiting the scope of the optimisation to cases where both the freeze and the load value have a use of one. I hope that's ok! If we find multiple-use cases in future that would be useful to optimise we can always extend this?

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