[PATCH] D70595: [TargetLowering] Allow constants with multiple uses

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 3 11:10:44 PST 2019


spatel added a comment.

First, we can definitely reduce the test - it only needs an fneg(fma(fmul X, C), -C, Y) pattern to show the crash, and that should repro on any target with fma support (so it's just unlucky that we didn't hit this bug sooner).

I thought we could get away with a simpler patch as originally proposed here, but I have convinced myself that would only be sweeping the bug under the rug. In fact, I don't think the current version of this patch goes far enough. When we're rewriting the expression, we can create arbitrary values, so *any* value (not just a constant) could have its number of uses changed and cause a crash/assert from mismatched handling in isNegatibleForFree/getNegatedExpression. I haven't reduced a test for that, but it seems possible.

I'll post an alternate patch that should solve the problem and see what people think of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70595





More information about the llvm-commits mailing list