[llvm] [InstCombine] Remove Store when its Ptr is removable (PR #79565)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Wed May 8 17:35:16 PDT 2024


nikic wrote:

> > This issue is Won't Fix as far as I'm concerned. Reaching a fix-point in _all_ cases is an explicit non-goal -- the verification exists to catch bugs like failing to add things back to the worklist.
> 
> If this is the case, I think the current behavior of error-by-default isn't really reasonable. You should be able to run instcombine standalone without triggering errors like this

Our requirement is that fix-point verification is enabled by default when writing instcombine tests, to prevent accidental introduction of worklist management bugs. The way we currently do this is to make `-passes=instcombine` enable fix-point verification, as that's what we use in tests. When instantiating `InstCombinePass()` from C++ code we disable it by default, as the verification should not be enabled in any production use.

Those are basically the constraints here -- happy to hear alternative ways we could have this "verify in tests, don't verify in production" behavior. Possibly the error message just needs to be improved to make it clear that this is not necessary a problem?

> Put up a WIP https://github.com/arsenm/llvm-project/tree/issue68120-worklist-issue

This will miscompile cases if there are multiple uses along the stripped chain, I think?

https://github.com/llvm/llvm-project/pull/79565


More information about the llvm-commits mailing list