[PATCH] D105009: [LSR] Handle case 1*reg => reg. PR50918

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 29 21:04:35 PDT 2021


mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:514
     return;
-  // So far we did not need this case. This is easy to implement but it is
-  // useless to maintain dead code. Beside it could hurt compile time.
----------------
reames wrote:
> Given the comment indicates an unimplemented case here, why not just implement it?
> 
> Adding the following code just above the assert appears to address your test case.
> 
> +
> +  if (BaseRegs.empty() && ScaledReg && Scale == 1) {
> +    BaseRegs.push_back(ScaledReg);
> +    Scale = 0;
> +    ScaledReg = nullptr;
> +    return;
> +  }
> 
> Any reason why this is a poor approach?
It seems that null scaled reg is not allowed, because this code always sets it to non-null.
```
  if (!ScaledReg) {
    ScaledReg = BaseRegs.pop_back_val();
    Scale = 1;
  }
```
I don't know if there are other places that rely on this.


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

https://reviews.llvm.org/D105009



More information about the llvm-commits mailing list