This is an archive of the discontinued LLVM Phabricator instance.

[LSR] Canonicalize a formula before insert it into the list
ClosedPublic

Authored by mdchen on Sep 1 2020, 7:15 AM.

Details

Summary

In GenerateConstantOffsetsImpl, we may generate non canonical
Formula if BaseRegs of that Formula is updated and includes a
recurrent expr reg related with current loop while its ScaledReg
is not.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=47329

Diff Detail

Event Timeline

mdchen created this revision.Sep 1 2020, 7:15 AM
mdchen requested review of this revision.Sep 1 2020, 7:15 AM
mdchen updated this revision to Diff 289433.Sep 2 2020, 6:28 AM

add a test case

qcolombet accepted this revision.Sep 2 2020, 12:01 PM

LGTM, nitpicks below.

llvm/test/Transforms/LoopStrengthReduce/AArch64/pr47329.ll
1 ↗(On Diff #289433)

Could you add a check that we generate something sensible?
(Not crashing is not a good enough check :))

22 ↗(On Diff #289433)

Please get rid of the implicit variables (%[0-9]+). (You can use opt -instnamer for that.)

This revision is now accepted and ready to land.Sep 2 2020, 12:01 PM
mdchen added inline comments.Sep 2 2020, 8:06 PM
llvm/test/Transforms/LoopStrengthReduce/AArch64/pr47329.ll
1 ↗(On Diff #289433)

I agree it's not good enough. However, the IR generated with assertions turned off is no difference with the one generated with this fix patch and I can't come up with reasonable checks here. I'd appreciate it if you can give some guidance :)

This revision was automatically updated to reflect the committed changes.