[PATCH] D151394: [LSR] Treat URem as uninteresting

[Peixin Qiao via Phabricator via llvm-commits]

peixin added a comment.

In D151394#4372286 <https://reviews.llvm.org/D151394#4372286>, @nikic wrote:

> Can you please explain in more detail what causes the miscompile in the first place?

Sure.

Actually, this bug should exist long time ago. The bug is exposed when we enable opaque pointer and the IR generated becomes different. It seems the old IR with typed pointer cannot trigger this scenario.

In this scenario, there are two phi nodes with i64 and i32 type, and the use of phi node with i32 type is URem instruction. In LSR, the phi nodes are reduced into one with i64 type, and the urem can be replaced with udiv/add instructions in Scalar Evolution. In the above test case (urem-use-type-conversion), the phi node matters is `%dec.137`. When converting to i64, the negative value will be the problem. Check the following:

  // The wrong transformation: (think about what if %dec.137 is -1)
  %dec.137 = ... ; i64 type
  %rem = urem i64 %dec.137, 53
  %newrem = trunc i64 %rem to i32
  %call = ... %rem
  // The correct transformation:
  %dec.137 = ... ; i64 type
  %dec.137.2 = trunc i64 %dec.137 to i32
  %rem = urem i32 %dec.137.2, 53
  %call = ... %rem

Previously, when getting the IVuses, `isInteresting` thinks URem instruction is not one user of Phi node (%dec.137) since the SCEV expression of URem is Add expr. As a result, the user of Phi node (%dec.137) is analyzed as `%call`. Then, in function `LSRInstance::GenerateAllReuseFormulae` in LSR, the wrong transformation SCEV expression is generated.

@nikic  Please let me know if something is not clear to you.
For example, there is UDiv SCEV, so the bug does exist when udiv is the user of Phi node (%dec.137).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151394