llvm dev - Apr 2017 - [InstCombine] When should InstCombine indicate something was changed to the pass managers?

InstCombine consists of iteration of the following two steps

1. Building the worklist over the function. Including dead code
elimination, constant folding, and constant argument folding.
2. Processing the worklist.

InstCombine currently only reports a change to the pass manager if more
than 1 iteration is executed. And we only do additional iterations when
worklist processing changed something.

But the first iteration could do deadcode elmination or constant folding
during the worklist build, but make no changes during the processing. Do
those count as changes that should be passed up to the pass manager?

There's an additional odd quirk here, if we constant fold an argument
during the worklist build we do return a changed flag from the worklist
build which gets ORed with the change flag for the worklist processing.
This can force another iteration even if the worklist processing itself
doesn't make any change. I've seen this happen during some InstCombine
runs
where we constant fold a GEP ConstantExpr just to change an i32 to i64 for
one of the indices.Clearly we shouldn't be forcing a subsequent iteration
just because the worklist build changed something but the worklist itself
didn't.

Thanks,
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170404/bb4a9887/attachment.html>

On Tue, Apr 4, 2017 at 12:56 PM, Craig Topper via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> InstCombine consists of iteration of the following two steps
>
> 1. Building the worklist over the function. Including dead code
elimination,
> constant folding, and constant argument folding.
> 2. Processing the worklist.
>
> InstCombine currently only reports a change to the pass manager if more
than
> 1 iteration is executed. And we only do additional iterations when worklist
> processing changed something.
>
> But the first iteration could do deadcode elmination or constant folding
> during the worklist build, but make no changes during the processing. Do
> those count as changes that should be passed up to the pass manager?
>
In the current world, the semantic associated to the return value of a
pass execution is, IMHO: "did this pass modify the unit of IR on which
is working on?"
Folding + removing instructions have visible effects, so, yes, I think
it should return `true`.
The current logic has the unfortunate effect of invalidating all the
analyses. When the new pass manager will be in place, you may be a
little bit more granular as the return value of a pass is a Preserved
set of analyses rather than a boolean value.
> There's an additional odd quirk here, if we constant fold an argument
during
> the worklist build we do return a changed flag from the worklist build
which
> gets ORed with the change flag for the worklist processing. This can force
> another iteration even if the worklist processing itself doesn't make
any
> change. I've seen this happen during some InstCombine runs where we
constant
> fold a GEP ConstantExpr just to change an i32 to i64 for one of the
> indices.Clearly we shouldn't be forcing a subsequent iteration just
because
> the worklist build changed something but the worklist itself didn't.
>
Maybe this can be improved, although I think one iteration more hardly
matters for compile time, YMMV.

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare