[PATCH] D85159: [ConstProp] Remove ConstantPropagation

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/test/Analysis/ConstantFolding/vscale.ll:210
-;
-  %i1 = insertelement <vscale x 4 x i32> undef, i32 1, i32 0
-  %i2 = shufflevector <vscale x 4 x i32> %i1, <vscale x 4 x i32> undef, <vscale x 4 x i32> zeroinitializer
----------------
nikic wrote:
> aeubanks wrote:
> > fhahn wrote:
> > > why are those removed? Is that functionality missing from instsimplify?
> > Looks like in ConstProp it always calls `ConstantFoldInstruction()`, which in this case just returns the same value, and replaces all uses, inlining into the ret. But InstSimplify calls `SimplifyInstruction()` which special cases vector instructions but doesn't know how to simplify these values any more, so it returns nullptr and InstSimplify doesn't do anything.
> > 
> > Maybe inlining these values into the ret isn't super useful?
> Sounds like an InstSimplify bug. SimplifyInstruction is supposed to be a superset of ConstantFoldInstruction.
I took a look at this. It seems like insertelement constants aren't handled as well as insertelement instructions.

For example, at [1], an instruction is properly handled, causing 
```
define i32 @foo() {
  %v = insertelement <vscale x 4 x i32> undef, i32 1, i64 4
  %r = extractelement <vscale x 4 x i32> %v, i64 4
  ret i32 %r
}
```
to become
```
define i32 @foo() {
  ret i32 1
}
```
But if we start inlining vector constants then it ends up at
```
define i32 @foo() {
  ret i32 extractelement (<vscale x 4 x i32> insertelement (<vscale x 4 x i32> undef, i32 1, i64 4), i64 4)
}
```
since `llvm::findScalarElement` doesn't properly handle insertelement constants.

This can of course be fixed (I'm struggling to figure out how to peek into an `InsertElementConstantExpr` though...), but I wonder about the extra work of handling both insertelement instructions and constants everywhere and how maintainable that is.

Thoughts?

[1]: https://github.com/llvm/llvm-project/blob/fb04d7b4a69831f6b999b1776da738557b108e0d/llvm/lib/Analysis/VectorUtils.cpp#L282


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85159