[SCEV] Support SMin/Umin for GetMinTrailingZeros

[caojoshua]

Summary

There is already support for Max counterparts. No reason why Min should
not be supported.

NFC: code in GetMinTrailingZeroes is copied for a couple node types.
Refactor them into a single code block.

[caojoshua]

This comment was removed by caojoshua.

[caojoshua]

edit comment in test

[caojoshua]

Add a test where trip multiple is 1

[nikic]

llvm/lib/Analysis/ScalarEvolution.cpp

6342

While here, would you mind supporting UMinSeq as well?

6346–6347

Any reason to deviate from the previous structure?

[caojoshua]

llvm/lib/Analysis/ScalarEvolution.cpp

6342

Yeah sure. Let me test this later. I think we can simplify this to

isa<SCEVMinMaxExpr> || isa<SCEVSequentialMinMaxExpr>

6346–6347

i -> I because LLVM coding standards state that Variable names should be nouns (as they represent state). The name should be camel case, and start with an upper case letter (e.g. Leader or Boats). And cause my editor is smart enough to know we are using LLVM coding standards and recommend this change.

No need to assign assign getNumOperands() to E. Can just compare directly against getNumOperands(), and remove unnecessary E variable. M is const, so we know the number of operands will not change through iterations barring some unsafe casting (I think this is how C++ const works).

No strong opinions from me. This deviates from code style in rest of function. I can update the rest of the function as a NFC followup to make sure everything is consistent.

[caojoshua]

• support UMinSequential. Add a test (see note below)
• single isa<SCEVMinMaxExpr>(S) instead of checking each subtype
• fix code style based on comments

I am unable to create a testcase for GetMinTrailingZeroes on UMinSequential
SCEVs. I tried adapting some tests from https://reviews.llvm.org/D116766, but
however I modify tests, either the Trip Multiple is not returning what I would
expect, or the UMinSequential turns into a regular umin.

All existing integration tests pass. Not sure if we need more testing, or we
can support UMinSequential in a separate work.

[caojoshua]

llvm/lib/Analysis/ScalarEvolution.cpp

6346–6347

I reverted the previous structure, but kept the variable names capitalized

[nikic]

LGTM