[PATCH] D122457: [LSR] Canonicalize formula before inserting it.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

Interesting note. It seems that the definition of canonical representation of the formula looks as follows:

  /// The list of "base" registers for this use. When this is non-empty. The
  /// canonical representation of a formula is
  /// 1. BaseRegs.size > 1 implies ScaledReg != NULL and
  /// 2. ScaledReg != NULL implies Scale != 1 || !BaseRegs.empty().
  /// 3. The reg containing recurrent expr related with currect loop in the
  /// formula should be put in the ScaledReg.
  /// #1 enforces that the scaled register is always used when at least two
  /// registers are needed by the formula: e.g., reg1 + reg2 is reg1 + 1 * reg2.
  /// #2 enforces that 1 * reg is reg.
  /// #3 ensures invariant regs with respect to current loop can be combined
  /// together in LSR codegen.
  /// This invariant can be temporarily broken while building a formula.
  /// However, every formula inserted into the LSRInstance must be in canonical
  /// form.

The important statement here  is "reg **containing** recurrent expr" while isCanonical utility checks that reg **is** recurrent expr.
The utility that makes canonicalization also operates on reg as if it **is** recurrent expr but not **contains**.

Might be the bug is here.


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

https://reviews.llvm.org/D122457