[PATCH] D39637: [GVN PRE] Patch the source for Phi node in PRE

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 8 15:32:50 PST 2017


efriedma added a comment.

> If we take the example for jump threading, and make it go through a load (where the load has the value of the two adds), and then replace the load with a phi of the two values, is there a reason we don't have the same problem?

GVN can do a few different kinds of replacements.  It can replace a scalar operation with the value of a different scalar operation.  It can replace a load with the value of a different load.  Or it can replace the value of the load with an operand of a store.

The problem the call to andIRFlags solves is an issue with replacing a scalar operation with a different scalar operation: "nsw" isn't passed into the value numbering, so we sometimes end up replacing an operation which can't produce poison with an operation which can produce poison.  For example, GVN can replace "add i32 %x, i32 %y" with "add nsw i32 %x, i32 %y". The latter produces poison in some cases, so it isn't precisely equivalent.  So when we perform the replacement, we need to erase the nsw.

If we're replacing a load with the operand of a store, we do not want to call andIRFlags; the original IR flags are fine, and "and"ing together the flags for two different operations doesn't make sense. We check `isa<LoadInst>` to catch this case.

Of course, if you're trying to some other, more complicated replacement, this logic could break down.


https://reviews.llvm.org/D39637





More information about the llvm-commits mailing list