[llvm-commits] [llvm] r67180 - in /llvm/trunk: lib/Transforms/Utils/Local.cpp test/Transforms/ScalarRepl/2009-03-17-CleanUp.ll

[Duncan Sands]

Hi Zhou,

> Sorry for the unclear comments  -:)
> 
> This method "OnlyUsedByDbgInfoIntrinsics()" is used by pass scalarrepl, and
> if it returns true,
> the pass scalarrepl would delete the instructions that only used by dbginfo.
> 
> However, we should consider one corner situation:  if the instruction
> visited has no users, for example, storing some value to memory, it should
> return false, otherwise, pass scalarrepl would incorrectly delete that store
> instruction which is dangerous.

what if the instruction writes to memory and has users, and those users
are only dbginfo intrinsics?  For example, it might be a call instruction
that modifies memory.  Then it will still be (wrongly) deleted.  Maybe this
case is impossible (I don't know what kinds of things can be used by dbginfo
intrinsics), but I still think the logic of your change is wrong: it seems to
me that OnlyUsedByDbgInfoIntrinsics should mean "if it wasn't for debug
intrinsics, this instruction would have no uses".  Then any transformations
based on it had better be correct for instructions with no uses.  It seems
logical to me that it should return "true" when there are no uses.  If some
user of OnlyUsedByDbgInfoIntrinsics has logic that would be wrong when there
are no uses, it's likely to be wrong when there are only debug uses too.
Anyway, I think this should be fixed in scalarrepl, not in
OnlyUsedByDbgInfoIntrinsics.

Ciao,

Duncan.