[PATCH] D45842: [Reassociate] swap binop operands to increase factoring potential

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/Reassociate/matching-binops.ll:297
 
-; Verify that debug info for modified instructions gets discarded (references become undef).
+; Verify that debug info for modified instructions is not invalid.
 
----------------
aprantl wrote:
> bjope wrote:
> > nit: Verify that we "discard" the dbg.value for variable "a" (`metadata !19` in the input, `metadata !18` in the output), since we do nto calculate the value `%and` after the transformation.
> > 
> > @aprantl once told me that is was better to use `metadata i32 undef` instead of `metadata !{}` when a dbg.value is "discarded". I think it is out-of-scope for this patch, but maybe the code that picks `metadata !2` in this solution should insert an undef value instead (or maybe later passes should handle `metadata !{}` the same way as if we have an explicit undef value, in case there really is a difference today).
> Do you think it would help to have a common facility along the lines of DbgInstrinsicInstr::replaceWithUndef() to make it easier to do the right thing?
Definitely. I don't know anything about debuginfo, so I don't know why one option is better than the other or what is best.

As you can see, I'm just using a default IRBuilder, so can we get it to do the optimal thing automatically?


https://reviews.llvm.org/D45842