Doerfert, Johannes Rudolf via llvm-dev
2018-Dec-12 00:51 UTC
[llvm-dev] The bit pattern after stripPointerCasts()
Hi, in a recent review [0], Florian Hahn helped me to realize something that was rather surprising to me: The widely popular and very useful function llvm::Value::stripPointerCasts() can return a value with a different bit pattern than the input. Now, I think this should not be the case but I want the hear other opinions. Before I go on, please not that there is at least one location in the code base [1] that makes a wrong assumption about the bit pattern preservation. The Problem ----------- StripPointerCasts will peel of address space casts but these can potentially change the bit pattern. Possible Solutions ------------------ 1) Make stripPointerCasts not look through address space casts. A new function (or parameter) would then be introduced to do the same thing _and_ peel of address space casts. 2) Keep looking through address space casts in stripPointerCasts. A new function (or parameter) would then be introduced to do the same but with the guarantee that the bit pattern is preserved. Note that the module implementation of stripPointerCasts and friends makes basically all the changes to them easy. Solution one has the benefit that our code is "more sound" than before because the stripPointerCasts function is more restrictive. We might lose performance though. Solution two will not disturb any existing code but we might need to revisit the uses of stripPointerCasts to fix existing bugs. Suggestions welcome! Thanks, Johannes [0] https://reviews.llvm.org/D54956?vs=on&id=175572&whitespace=ignore-most#inline-485807 [1] LazyValueInfo::getPredicateAt() line ~1714, isKnownNull(...) is used after the stripPointerCasts() function might have changed the bit pattern (and thereby the null-ness). -- Johannes Doerfert Researcher Argonne National Laboratory Lemont, IL 60439, USA jdoerfert at anl.gov -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181212/8ae3993e/attachment.sig>
Finkel, Hal J. via llvm-dev
2018-Dec-14 03:21 UTC
[llvm-dev] The bit pattern after stripPointerCasts()
On 12/11/18 6:51 PM, Doerfert, Johannes Rudolf via llvm-dev wrote: Hi, in a recent review [0], Florian Hahn helped me to realize something that was rather surprising to me: The widely popular and very useful function llvm::Value::stripPointerCasts() can return a value with a different bit pattern than the input. Now, I think this should not be the case but I want the hear other opinions. Before I go on, please not that there is at least one location in the code base [1] that makes a wrong assumption about the bit pattern preservation. If there's really only one place that gets this wrong (or only a few), I'm inclined to suggest option (1) below and fix this one place. Matt, any thoughts on this? -Hal The Problem ----------- StripPointerCasts will peel of address space casts but these can potentially change the bit pattern. Possible Solutions ------------------ 1) Make stripPointerCasts not look through address space casts. A new function (or parameter) would then be introduced to do the same thing _and_ peel of address space casts. 2) Keep looking through address space casts in stripPointerCasts. A new function (or parameter) would then be introduced to do the same but with the guarantee that the bit pattern is preserved. Note that the module implementation of stripPointerCasts and friends makes basically all the changes to them easy. Solution one has the benefit that our code is "more sound" than before because the stripPointerCasts function is more restrictive. We might lose performance though. Solution two will not disturb any existing code but we might need to revisit the uses of stripPointerCasts to fix existing bugs. Suggestions welcome! Thanks, Johannes [0] https://reviews.llvm.org/D54956?vs=on&id=175572&whitespace=ignore-most#inline-485807 [1] LazyValueInfo::getPredicateAt() line ~1714, isKnownNull(...) is used after the stripPointerCasts() function might have changed the bit pattern (and thereby the null-ness). _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181214/95b82a7b/attachment.html>
Matt Arsenault via llvm-dev
2018-Dec-14 03:40 UTC
[llvm-dev] The bit pattern after stripPointerCasts()
> On Dec 14, 2018, at 2:21 PM, Finkel, Hal J. via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > > On 12/11/18 6:51 PM, Doerfert, Johannes Rudolf via llvm-dev wrote: >> Hi, >> >> in a recent review [0], Florian Hahn helped me to realize something that >> was rather surprising to me: >> >> The widely popular and very useful function >> llvm::Value::stripPointerCasts() can return a value with a different >> bit pattern than the input. >> >> Now, I think this should not be the case but I want the hear other >> opinions. Before I go on, please not that there is at least one location >> in the code base [1] that makes a wrong assumption about the bit pattern >> preservation. > If there's really only one place that gets this wrong (or only a few), I'm inclined to suggest option (1) below and fix this one place. > > Matt, any thoughts on this? > > -Hal > >Yes, I’ve wanted a version which strips addrspacecast and which doesn’t before, so +1 -Matt -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181214/1676f428/attachment.html>