[PATCH] D84664: [InstCombine] Fold shift-select if all the operands are constant

[Matteo Favaro via Phabricator via llvm-commits]

fvrmatteo added a comment.

Adding more context to the missed optimisation.

**FoldShiftByConstant** is calling **canEvaluateShifted** and internally that's checking if the instruction has a single use:

  // We can't mutate something that has multiple uses: doing so would
  // require duplicating the instruction in general, which isn't profitable.
  if (!I->hasOneUse()) return false;

This is the check that lets the optimisation to be properly applied when a single use is present and blocks it when more uses are present. I therefore reverted my changes and tried to disable that check, resulting in the optimisation being applied properly:

  ICE: GetShiftedValue propagating shift through expression to eliminate shift:
    IN:   %v0 = select i1 %arg, i64 64, i64 0
    SH:   %v1 = lshr exact i64 %v0, 6
  IC: ADD:   %v0 = select i1 %arg, i64 64, i64 0
  IC: Replacing   %v1 = lshr exact i64 %v0, 6
      with   %v0 = select i1 %arg, i64 1, i64 0

But resulting in a final LLVM-IR which is wrong <https://alive2.llvm.org/ce/z/ZF5LLt>:

  ; ModuleID = 'lshr-select-const.ll'
  source_filename = "lshr-select-const.ll"
  
  define i64 @fold_lshr(i1 %arg) {
  entry:
    %v0 = zext i1 %arg to i64
    %0 = or i64 %v0, 8589934590
    %v2.neg = add nuw nsw i64 %0, 1
    %v6 = and i64 %v2.neg, 5369111591
    ret i64 %v6
  }

I think that's the **hasOneUse** call that we are looking for, but simply disabling it is a non-solution because we run into wrong code and we need to take into account the instruction duplication.

Although I think checking for constant operands before checking for **hasOneUse** may be necessary because that's not going to result into duplication.

It's my first investigation of a missed optimisation, I'm sorry for the increasing context.


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

https://reviews.llvm.org/D84664