Miscompile with opt -passes="loop-unroll,loop-mssa(licm,indvars)" -unroll-count=4

[mikaelholmen]

llvm commit: e76b257
Reproduce with:
opt -passes="loop-unroll,loop-mssa(licm,indvars)" -unroll-count=4 bbi-95405.ll -S -o -

The input function returns 32768, but after running the passes as above we get

define i16 @foo() {
entry:
  br label %loop

loop:                                             ; preds = %loop, %entry
  br i1 false, label %loop, label %end, !llvm.loop !0

end:                                              ; preds = %loop
  ret i16 8192
}

!0 = distinct !{!0, !1}
!1 = !{!"llvm.loop.unroll.disable"}

So now the function returns 8192 instead.

If I extract the IR after loop-unroll or licm and run the rest of the passes instead I get the correct result.

bbi-95405.ll.gz

[mikaelholmen]

This starts happening with #67736 , commit 2dd5204

Recommit "[LICM] Support integer mul/add in hoistFPAssociation. (#67736)"

With a fix for build bot failure. I was accessing the type of a deleted
Instruction.

Original message:

The reassociation this is trying to repair can happen for integer types
too.

This patch adds support for integer mul/add to hoistFPAssociation. The
function has been renamed to hoistMulAddAssociation. I've used separate
statistics and limits for integer to allow tuning flexibility.

[nikic]

Without looking into this in detail, it's possible that the way the LICM code modifies instructions in place results incorrect cached SCEV results.

[mikaelholmen]

Without looking into this in detail, it's possible that the way the LICM code modifies instructions in place results incorrect cached SCEV results.

I added some printouts and see that when IndVarSimplify run rewriteLoopExitValues we do

const SCEV *ExitValue = SE->getSCEVAtScope(Inst, L->getParentLoop());

for Inst being

%mul.n = mul i16 %mul, 8

and then we get the SCEV

(2 * %mul)

which looks wrong.

If I split up the pipeline in several opt runs we instead get the SCEV

(8 * %mul)

which looks more reasonable (and then we also get the correct result).
LICM changed

%mul.n = mul i16 %mul, 2

to

%mul.n = mul i16 %mul, 8

so yeah, it should perhaps do something (more) about SCEV.

[mikaelholmen]

@antoniofrighetto : nice that you work on this!

But oh it's annoying that I don't get any emails from github about that there was activity here with a linked PR with a fix etc even if I wrote this issue. :( I do get emails when there are comments but not when the issue was assigned or the PR was created. :(

[antoniofrighetto]

@mikaelholmen This is a bit inconvenient indeed, GH doesn’t seem to send notifications wrt that :( Closed this, thanks for reporting!

[mikaelholmen]

@mikaelholmen This is a bit inconvenient indeed, GH doesn’t seem to send notifications wrt that :( Closed this, thanks for reporting!

Thank you!