[PATCH] D61607: Introduce an option to stripPointerCasts to force the same bit pattern

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: llvm/include/llvm/IR/Value.h:504
+  const Value *stripPointerCasts(bool KeepBitPattern = false) const;
+  Value *stripPointerCasts(bool KeepBitPattern = false) {
     return const_cast<Value *>(
----------------
jdoerfert wrote:
> arsenm wrote:
> > I think a separate function name would be more helpful, implemented with a hidden bool argument 
> It depends on different things, including taste. I don't feel strong so I'd be fine with a new function as well. Two things to consider though:
> 
> Do we want to prevent as-cast stripping in other strip-casts scenarios as well?
> This patch enables it for PSK_ZeroIndicesAndAliases but not for PSK_ZeroIndices and  PSK_ZeroIndicesAndAliasesAndInvariantGroups. If the answer is yes, then we would end up with even more versions in the stripXXXX family (we have a bunch already).
> 
> Do we want to be able to set `KeepBitPattern` as the default in order for us to "guarantee" correctness and give people a way to opt-in? We can achieve this also if the new function would look through as-casts and `stripPointerCasts` wouldn't anymore.
> 
> 
> 
If we want to be able to switch the default at some point, I certainly recommend something with a name so that we can find the various forms with grep (etc.). We can always add more forms as needed (or we can switch the argument type to an enum).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61607