[PATCH] D71450: Modification of bad code in Reassociate.cpp to avoid potential bug.

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

If I understand correctly, the problem is that calling user_back() might access a container with zero elements, yes? This would appear to be an easy fix -- however IMO you should add reviewers who're familiar with the Reassociate pass better [0]. From a couple of minutes examination it would appear `ShouldBreakUpSubtract` is only callable from OptimizeInst, and that in turn is guarded by `isInstructionTriviallyDead`, which I would expect to skip such an instruction. There might be something more complicated broken that someone more familiar with reassociate might spot.

If this really does cause a crash, a regression test would be in order too.

[0] Maybe you have already, I don't recognise all the names, sorry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71450